Re: Unaligned writes on the kexec path

[Simon Horman <horms@verge.net.au>]

On Tue, Nov 29, 2011 at 12:54:09PM +0000, Andrew Cooper wrote:
> On 29/11/11 11:35, Jan Beulich wrote:
> >>>> On 29.11.11 at 11:49, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> On 29/11/11 05:51, Simon Horman wrote:
> >>> Hi Keir, Hi Andrew,
> >>>
> >>> On Mon, Nov 28, 2011 at 08:42:16PM +0000, Keir Fraser wrote:
> >>>> On 28/11/2011 14:24, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:
> >>>>
> >>>>> Hello,
> >>>>>
> >>>>> In c/s 7493bb48d89f, you change the internals of kexec_crash_save_info()
> >>>> The patch is by Simon Horman, cc'ed, not me.
> >> Right.  Sorry.  I should have remembered that basing "who wrote the
> >> patch" on a simple hg log was not a safe bet.
> > I don't think there's much room for improvement - all members of
> > crash_xen_info_t are of "unsigned long" type, but ELF note handling
> > will only ever guarantee 4-byte alignment.
> >
> > Jan
> >
> Depending on how flexible we want to be, we can either specify that the
> name field should be 2n words long plus 1-4 bytes, which will cause it
> to align to an odd number of 4 bytes, which will cause the desc field to
> be aligned to 8 bytes when the type field in the note header is taken
> into account.  Then, the desc field should be constrained to be (2n+1) +
> 1-4bytes which would cause it to have 8 byte alignment, and subsequently
> 8 byte align the next note.
> 
> Alternatively, we could artificially extend the name up to an odd 4 byte
> alignment, and desc field up to 8 byte alignment with trailing \0's and
> include this as part of their length fields.  All names should be
> processed as Null terminating strings (which wont suffer from having
> extra Nulls at the end) and I have yet to see processing of a note which
> doesn't take the buffer and cast it to a structure pointer.  This also
> wont suffer from from trailing data.
> 
> Then again, this does sound like quite a lot of work for not a lot, and
> there is no guarantee that we wont break some of the more special code
> which works with elf files in 'special' ways.

I believe that the scheme you suggest would work. But elf parsing
does tend to be a bit special. So I lean towards not changing things.

> (What really should have happened was for ELF64 to specify 64bit
> alignment of things like this, but we live and learn)

Agreed.