----- Original Message -----
> 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?

Good question.  Because of the way it's declared as a section, I don't
think so.  But your section_size() static is fine -- just remove the 
comment at the top...  ;-)

Thanks,
  Dave

> 
> >> ---
> >>  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