[PATCH v3 01/21] vmcore: reference e_phoff member explicitly to get position of program header table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com> writes:

> From: "Eric W. Biederman" <ebiederm at xmission.com>
> Subject: Re: [PATCH v3 01/21] vmcore: reference e_phoff member explicitly to get position of program header table
> Date: Tue, 19 Mar 2013 14:44:16 -0700
>
>> HATAYAMA Daisuke <d.hatayama at jp.fujitsu.com> writes:
>> 
>>> Currently, the code assumes that position of program header table is
>>> next to ELF header. But future change can break the assumption on
>>> kexec-tools and the 1st kernel. To avoid worst case, reference e_phoff
>>> member explicitly to get position of program header table in
>>> file-offset.
>> 
>> In principle this looks good.  However when I read this it looks like
>> you are going a little too far.
>> 
>> You are changing not only the reading of the supplied headers, but
>> you are changing the generation of the new new headers that describe
>> the data provided by /proc/vmcore.
>> 
>> I get lost in following this after you mangle merge_note_headers.
>> 
>> In principle removing silly assumptions seems reasonable, but I think
>> it is completely orthogonal to the task of maping vmcore mmapable.
>> 
>> I think it is fine to claim that the assumptions made here in vmcore are
>> part of the kexec on panic ABI at this point, which would generally make
>> this change unnecessary.
>
> This was suggested by Vivek. He prefers generic one.
>
> Vivek, do you agree to this? Or is it better to re-post this and other
> clean-up patches as another one separately to this patch set?

My deep problem with this patch was that you were changing how we think
about the generated headers, and that just didn't seem to make any
sense, and certainly it seems to be orthogonal from the patch itself.

For headers that we generate it is perfectly fine to make all kinds of
assumptions.

Eric




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux