Re: [Xen-devel] [PATCH v1 3/3] xen/livepatch/ARM32: Don't crash on livepatches loaded with wrong alignment.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 07/11/17 10:34 PM >>>
>On Tue, Jul 11, 2017 at 02:06:09PM -0600, Jan Beulich wrote:
>> >>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 07/11/17 6:53 PM >>>
>> >This issue was observed on ARM32 with a cross compiler for the
>> >livepatches. Mainly the livepatches .data section size was not
>> >aligned to the section alignment:
>> >
>> >ARM32 native:
>> >Contents of section .rodata:
>>  >0000 68695f66 756e6300 63686563 6b5f666e  hi_func.check_fn
>>  >0010 63000000 78656e5f 65787472 615f7665  c...xen_extra_ve
>>  >0020 7273696f 6e000000                    rsion...
>> >
>> >ARM32 cross compiler:
>> >Contents of section .rodata:
>>  >0000 68695f66 756e6300 63686563 6b5f666e  hi_func.check_fn
>>  >0010 63000000 78656e5f 65787472 615f7665  c...xen_extra_ve
>>  >0020 7273696f 6e00                        rsion.
>> >
>> >And when we loaded it:
>> >
>> >native:
>> >
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 
>> >00a04024
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions 
>> >at 00a0404c
>> >
>> >cross compiler:
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .text at 00a02000
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .rodata at 
>> >00a04024
>> >(XEN) livepatch.c:413: livepatch: xen_hello_world: Loaded .altinstructions 
>> >at 00a0404a
>> >
>> >(See 4a vs 4c)
>> >
>> >native readelf:
>>   >[ 4] .rodata           PROGBITS        00000000 000164 000028 00   A  0   
>> 0  4
>>   >[ 5] .altinstructions  PROGBITS        00000000 00018c 00000c 00   A  0   
>> 0  1
>> >
>> >cross compiler readelf --sections:
>>   >[ 4] .rodata           PROGBITS        00000000 000164 000026 00   A  0   
>> 0  4
>>   >[ 5] .altinstructions  PROGBITS        00000000 00018a 00000c 00   A  0   
>> 0  1
>> >
>> >And as can be seen the .altinstructions have alignment of 1 which from
>> >'man elf' is: "Values of zero and one mean no alignment is required."
>> >which means we can ignore it.
>> >
>> >However ignoring this will result in a crash as when we started processing
>> >".rel.altinstructions" for ".altinstructions" with a cross-compiled payload
>> >we would end up poking in an section that was not aligned properly in memory
>> >and crash.
>> 
>> Which section is it that would not be aligned properly in memory?
>
>.altinstructions, thanks to .rodata not being padded properly.
>
>> .altinstructions, with an alignment of 1, can be placed anywhere. You
>> shouldn't enforce extra alignment. If higher alignment is needed, the
>> code producing this section emission needs to be fixed.
>
>And there is the path to madness :-) We would need to provide an
>linker map to make sure that they are with the correct alignment.

Why? I'd expect it to be the assembler directives creating contributions to
that section to simply lack a .align or alike. And indeed, there's nothing
like that in ARM's alternative.h. Please see commit 01fe4da624 ("x86: force
suitable alignment in sources rather than in linker script") for further
context.

Jan


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