[Crash-utility] Re: nr_cpus is not calculated properly

[Dave Anderson]

----- "Wei Jiang" <talk90091e at gmail.com> wrote:

> Hi,
> 
> I found nr_cpus is not calculated properly in 32 bits(x86) at
> crash-4.0-8.9.
> 
> Around line 140 in file lkcd_v8.c.
>    137           * to find out how many CPUs are configured.
>    138           */
>    139          offset = offsetof(dump_header_asm_t, dha_smp_regs[0]);
>    140          nr_cpus = (hdr_size - offset) / sizeof(dump_CPU_info_t);
>    141
>    142          fprintf(stderr, "CPU number NR_CPUS %d \n", NR_CPUS);
>    143          fprintf(stderr, "header_asm_t size %d \n",
>    sizeof(dump_header_asm_t));
>  
> And in the corresponding head file.
> # cat -n lkcd_dump_v8.h|grep -A 20 434
>    434          /* smp specific */
>    435          uint32_t        dha_smp_num_cpus;
>    436          uint32_t        dha_dumping_cpu;
>    437          struct pt_regs  dha_smp_regs[NR_CPUS];
>    438          uint32_t        dha_smp_current_task[NR_CPUS];
>    439          uint32_t        dha_stack[NR_CPUS];
>    440          uint32_t        dha_stack_ptr[NR_CPUS];
>    441  } __attribute__((packed)) dump_header_asm_t;
>    442
>    443  /*
>    444   * CPU specific part of dump_header_asm_t
>    445   */
>    446  typedef struct dump_CPU_info_s {
>    447          struct pt_regs  dha_smp_regs;
>    448          uint64_t        dha_smp_current_task;
>    449          uint64_t        dha_stack;
>    450          uint64_t        dha_stack_ptr;
>    451  } __attribute__ ((packed)) dump_CPU_info_t;
>    452
>    453
>    454  /*
> 
> As we know, on x86(32 bits), uint32_t is 4 bytes and uint64_t is 8
> bytes. 
> 
> So this line
>    140          nr_cpus = (hdr_size - offset) / sizeof(dump_CPU_info_t);
> 
> would not get a correct nr_cpus due to the sizeof().
> 
> A patch to fix this problem as below.

BTW, what exactly are the ramifications without the patch -- does the
crash session die during initialization?  How come nobody ran into 
this issue given that the code has been in place for almost 2 years?  

4.0-4.8  - ... 

         - Change for support of LKCD dumpfile version 8 and later to determine
           the backtrace starting registers from the dumpfile header.  Increase
           (maximum) NR_CPUS for ia64 to 4096.
           (bwalle at suse.de)

           ...

           (10/30/07)

Anyway, the patch looks reasonable to me, but I don't touch the LKCD
code without a sign-off from the LKCD maintainers on this mailing
list.

LKCD maintainers -- do you have any objection to this patch?

Thanks,
  Dave


> 
> Thanks.
> -Wj
> 
> --- lkcd_dump_v8.h.orig 2009-04-16 13:14:22.000000000 -0400
> +++ lkcd_dump_v8.h      2009-06-10 03:31:37.815122032 -0400
> @@ -445,9 +445,9 @@ typedef struct _dump_header_asm_s {
>   */
>  typedef struct dump_CPU_info_s {
>         struct pt_regs  dha_smp_regs;
> -       uint64_t        dha_smp_current_task;
> -       uint64_t        dha_stack;
> -       uint64_t        dha_stack_ptr;
> +       uint32_t        dha_smp_current_task;
> +       uint32_t        dha_stack;
> +       uint32_t        dha_stack_ptr;
>  } __attribute__ ((packed)) dump_CPU_info_t;