[Crash-utility] [PATCH v3 0/4] Generalize KASLR calculation and use it for KDUMPs

[Sergio Lopez]

On Tue, Mar 20, 2018 at 04:12:04PM -0400, Dave Anderson wrote:
> 
> Hi Sergio,  (and Takao and Daisuke)
> 
> This patch to map_cpus_to_prstatus_kdump_cmprs():
> 
> @@ -153,8 +154,13 @@ resize_note_pointers:
>                             dd->num_qemu_notes * sizeof(void *))) == NULL)
>                                 error(FATAL,
>                                     "compressed kdump: cannot realloc QEMU note pointers\n");
> +                       if  ((dd->nt_qemucs_percpu = realloc(dd->nt_qemucs_percpu,
> +                           dd->num_qemu_notes * sizeof(void *))) == NULL)
> +                               error(FATAL,
> +                                   "compressed kdump: cannot realloc QEMU note pointers\n");
>                 } else
>                         free(dd->nt_qemu_percpu);
> +                       free(dd->nt_qemucs_percpu);
>         }
>  }
> 
> is missing brackets around the else clause, resulting in:
> 
>         if (machine_type("X86_64") || machine_type("X86") ||
>             machine_type("ARM64")) {
>                 if ((dd->nt_prstatus_percpu = realloc(dd->nt_prstatus_percpu,
>                     dd->num_prstatus_notes * sizeof(void *))) == NULL)
>                         error(FATAL,
>                             "compressed kdump: cannot realloc NT_PRSTATUS note pointers\n");
>                 if (dd->num_qemu_notes) {
>                         if  ((dd->nt_qemu_percpu = realloc(dd->nt_qemu_percpu,
>                             dd->num_qemu_notes * sizeof(void *))) == NULL)
>                                 error(FATAL,
>                                     "compressed kdump: cannot realloc QEMU note pointers\n");
>                         if  ((dd->nt_qemucs_percpu = realloc(dd->nt_qemucs_percpu,
>                             dd->num_qemu_notes * sizeof(void *))) == NULL)
>                                 error(FATAL,
>                                     "compressed kdump: cannot realloc QEMU note pointers\n");
>                 } else
>                         free(dd->nt_qemu_percpu);
>                         free(dd->nt_qemucs_percpu);
>         }
> 
> So as soon as the realloc(dd->nt_qemucs_percpu, ...) is done, it gets free()'d.
> So I'm not sure how your testing worked, but presumably, the freed data 
> remained untouched at least long enough to get the job done during 
> initialization.

Ouch, thanks for catching this one. I'm not really used to mixing both
styles.

> One minor nit -- in order to be able to search for functions in a file 
> by entering "^function_name", I prefer that function names always be 
> placed on their own line, with the return type or void on the line above.  

Ack, will keep then in mind in the future.

Sergio.