Dave Anderson <ander...@redhat.com> writes:

> ----- Original Message -----
>> Since Xen commit 666aca08175b ("sched: use the auto-generated list of
>> schedulers") crash cannot open Xen vmcores, because symbol 'schedulers'
>> does not exits. Xen 4.7 implemented schedulers as its own section.
>> 
>> xen/arch/x86/xen.lds.S
>>        __start_schedulers_array = .;
>>        *(.data.schedulers)
>>        __end_schedulers_array = .;
>> 
>> Crash must not look up for "schedulers" symbol and fails, if symbol is
>> not found. It must check if __start_schedulers_array exits, and if it
>> does, use it and get size of the section. Otherwise, crash fallback to
>> looking up "schedulers" symbol. That way, crash can open vmcore before
>> and after Xen 4.7.
>> 
>> Signed-off-by: Nikola Pajkovsky <npajkov...@suse.cz>
>
> Hi Nikola,
>
> I'm not at at familiar with this code, but xen_hyper_schedule_init() declares
> scheduler_buf[] with a fixed size of XEN_HYPER_SCHEDULERS_ARRAY_CNT (10):
>
>   long schedulers_buf[XEN_HYPER_SCHEDULERS_ARRAY_CNT];
>
> If the new __start_schedulers_array size (and therefore 
> xen_hyper_nr_schedulers)
> could ever exceed 10, then there would be a buffer overflow when the buffer
> is filled.  So probably it would be best to dynamically allocate the buffer
> with GETBUF/FREEBUF, or at least limit it to 10?  Maybe 10 was picked as an
> absurdly large number that will never be reached?

That's good point and I will rework patch. Can get somehow size of
'schedulers' array?

>> ---
>>  xen_hyper.c | 40 +++++++++++++++++++++++++++++++++-------
>>  1 file changed, 33 insertions(+), 7 deletions(-)
>> 
>> diff --git a/xen_hyper.c b/xen_hyper.c
>> index 27e44c6e733d..e8268cf40c78 100644
>> --- a/xen_hyper.c
>> +++ b/xen_hyper.c
>> @@ -438,6 +438,19 @@ xen_hyper_misc_init(void)
>>  #define XEN_HYPER_SCHEDULERS_ARRAY_CNT 10
>>  #define XEN_HYPER_SCHEDULER_NAME 1024
>>  
>> +/*
>> + * does similar function exits in crash??
>> + */
>> +static int section_size(char *start_section, char *end_section)
>> +{
>> +    ulong sp_start, sp_end;
>> +
>> +    sp_start = symbol_value(start_section);
>> +    sp_end = symbol_value(end_section);
>> +
>> +    return (sp_end - sp_start) / sizeof(size_t);
>> +}
>> +
>>  static void
>>  xen_hyper_schedule_init(void)
>>  {
>> @@ -448,6 +461,8 @@ xen_hyper_schedule_init(void)
>>      char *buf;
>>      char opt_name_buf[XEN_HYPER_OPT_SCHED_SIZE];
>>      int i, cpuid, flag;
>> +    char *sp_name;
>> +    int xen_hyper_nr_schedulers;
>>  
>>      /* get scheduler information */
>>      if((xhscht->scheduler_struct =
>> @@ -469,21 +484,32 @@ xen_hyper_schedule_init(void)
>>      XEN_HYPER_OPT_SCHED_SIZE, "opt_sched,", RETURN_ON_ERROR)) {
>>              error(FATAL, "cannot read opt_sched,.\n");
>>      }
>> -    schedulers = symbol_value("schedulers");
>> +
>> +    if (symbol_exists("__start_schedulers_array")) {
>> +            sp_name = "__start_schedulers_array";
>> +            xen_hyper_nr_schedulers = 
>> section_size("__start_schedulers_array",
>> +                                                   
>> "__end_schedulers_array");
>> +    } else {
>> +            sp_name = "schedulers";
>> +            xen_hyper_nr_schedulers = XEN_HYPER_SCHEDULERS_ARRAY_CNT;
>> +    }
>> +
>> +    schedulers = symbol_value(sp_name);
>> +
>>      addr = schedulers;
>>      while (xhscht->name == NULL) {
>>              if (!readmem(addr, KVADDR, schedulers_buf,
>> -            sizeof(long) * XEN_HYPER_SCHEDULERS_ARRAY_CNT,
>> -            "schedulers", RETURN_ON_ERROR)) {
>> +                         sizeof(long) * xen_hyper_nr_schedulers,
>> +                         sp_name, RETURN_ON_ERROR)) {
>>                      error(FATAL, "cannot read schedulers.\n");
>>              }
>> -            for (i = 0; i < XEN_HYPER_SCHEDULERS_ARRAY_CNT; i++) {
>> +            for (i = 0; i < xen_hyper_nr_schedulers; i++) {
>>                      if (schedulers_buf[i] == 0) {
>>                              error(FATAL, "schedule data not found.\n");
>>                      }
>>                      if (!readmem(schedulers_buf[i], KVADDR,
>> -                    xhscht->scheduler_struct, XEN_HYPER_SIZE(scheduler),
>> -                    "scheduler", RETURN_ON_ERROR)) {
>> +                                 xhscht->scheduler_struct, 
>> XEN_HYPER_SIZE(scheduler),
>> +                                 "scheduler", RETURN_ON_ERROR)) {
>>                              error(FATAL, "cannot read scheduler.\n");
>>                      }
>>                      opt_name = ULONG(xhscht->scheduler_struct +
>> @@ -514,7 +540,7 @@ xen_hyper_schedule_init(void)
>>                      strncpy(xhscht->name, buf, strlen(buf));
>>                      break;
>>              }
>> -            addr += sizeof(long) * XEN_HYPER_SCHEDULERS_ARRAY_CNT;
>> +            addr += sizeof(long) * xen_hyper_nr_schedulers;
>>      }
>>      FREEBUF(buf);
>>  
>> --
>> 2.13.6
>> 
>> --
>> Crash-utility mailing list
>> Crash-utility@redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>> 

-- 
Nikola

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Reply via email to