Re: [PATCH v2 19/20] livepatch/elf: Adjust section aligment to word

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Aug 25, 2016 at 09:11:15AM -0600, Jan Beulich wrote:
> >>> On 25.08.16 at 15:37, <konrad.wilk@oracle.com> wrote:
> > On most architectures it does not matter what the aligment is.
> > 
> > On ARM32 it is paramount that the aligment is word-size (4)
> > otherwise we get a Data Abort when trying to perform ELF
> > relocations. That is due to ARM 32 only being able to write to
> > word-aligned addresses.
> 
> That's not exactly true, afaik: ARM can write to byte- and
> half-word-aligned addresses, but only bytes/half-words.
> 
> > --- a/xen/common/livepatch_elf.c
> > +++ b/xen/common/livepatch_elf.c
> > @@ -71,7 +71,15 @@ static int elf_resolve_sections(struct livepatch_elf *elf, const void *data)
> >          delta = elf->hdr->e_shoff + i * elf->hdr->e_shentsize;
> >  
> >          sec[i].sec = data + delta;
> > -
> > +        /*
> > +         * Some architectures REQUIRE section alignment to be word-size.
> > +         */
> 
> This is a single line comment.
> 
> > +        if ( sec[i].sec->sh_addralign % sizeof(uint32_t) )
> 
> Hmm, word size for ARM64 and x86-64 ought to be 8 bytes. Also you
> don't cover sec[i].sec->sh_addralign being zero (in fact any non-power-
> of-2 value would seem bogus to me). And then - why does this need to
> be done to all sections?

The issue I hit was relocations being done on .bug_frame section on ARM 32.

We had an .rel.bug_frame which would modify the .bug_frame and it would try
to use a uint32_t operation (R_ARM_REL32 if I recall correctly).
Since .bug_frame sh_addralign was 1, it ended up sadly with an
Data Abort exception.

The other sections that had .sh_addralign of 1 such as: livepatch.depends,
,.rodata.str1, and .strtab were OK - there was no problem with them having
byte alignment.

P.S.
On x86 and ARM64 the .bug_frame sections also have byte size alignment.

> 
> > +        {
> > +            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Adjusting aligment for section [%u]\n",
> > +                    elf->name, i);
> > +            ((Elf_Shdr *)sec[i].sec)->sh_addralign = 4;
> 
> And of course you know how I like such casting away of constness.

Heh.

Initially I was hoping the linker would have an --section-alignment like the ld PE one
which would make this a simple $LD_FLAGS change.

But without that the only other recourse I have (especially for the test-cases) is
to cobble up an livepatch.lds which I would need to sync with the xen.lds occassionaly.
That would guarantee that the livepatch'es would have the right section alignment and
then this patch would just return -EINVAL if the alignment was not to the power of 2.

Or something along this patch where we adjust it (and yes need to cast away the
constness).

Preferences?
> 
> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel