Linkers may put orphaned sections ahead of .text, which breaks the calling
requirements. A concrete example is Ubuntu's GCC-9 default of enabling
-fcf-protection which causes us to try and execute .note.gnu.properties during
Xen's boot.
Put .got.plt in its own section as it specifically needs preserving from the
linkers point of view, and discard everything else. This will hopefully be
more robust to other unexpected toolchain properties.
Fixes boot from an Ubuntu build of Xen.
Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Jason Andryuk <jandryuk@gmail.com>
CC: Stefan Bader <stefan.bader@canonical.com>
---
xen/arch/x86/boot/build32.lds | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds
index da35aee910..97454b40ff 100644
--- a/xen/arch/x86/boot/build32.lds
+++ b/xen/arch/x86/boot/build32.lds
@@ -31,7 +31,7 @@ SECTIONS
*(.bss.*)
}
- /DISCARD/ : {
+ .got.plt : {
/*
* PIC/PIE executable contains .got.plt section even if it is not linked
* with dynamic libraries. In such case it is just placeholder for
@@ -47,6 +47,14 @@ SECTIONS
*
* Please check build32.mk for more details.
*/
- /* *(.got.plt) */
+ *(.got.plt)
+ }
+
+ /DISCARD/ : {
+ /*
+ * Discard everything else, to prevent linkers from putting
+ * orphaned sections ahead of .text, which needs to be first.
+ */
+ *(*)
}
}
--
2.11.0
On Tue, May 12, 2020 at 3:11 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > Linkers may put orphaned sections ahead of .text, which breaks the calling > requirements. A concrete example is Ubuntu's GCC-9 default of enabling > -fcf-protection which causes us to try and execute .note.gnu.properties during > Xen's boot. > > Put .got.plt in its own section as it specifically needs preserving from the > linkers point of view, and discard everything else. This will hopefully be > more robust to other unexpected toolchain properties. > > Fixes boot from an Ubuntu build of Xen. > > Reported-by: Jason Andryuk <jandryuk@gmail.com> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Tested-by: Jason Andryuk <jandryuk@gmail.com> Thanks
On 12.05.2020 21:11, Andrew Cooper wrote:
> @@ -47,6 +47,14 @@ SECTIONS
> *
> * Please check build32.mk for more details.
> */
> - /* *(.got.plt) */
> + *(.got.plt)
> + }
> +
> + /DISCARD/ : {
> + /*
> + * Discard everything else, to prevent linkers from putting
> + * orphaned sections ahead of .text, which needs to be first.
> + */
> + *(*)
> }
> }
To be honest I'm not sure if this isn't going too far. Much
depends on what would happen if a new real section appeared
that needs retaining. Granted the linker may then once again
put it at the beginning of the image, and we'll be screwed
again, just like we'll be screwed if a section gets discarded
by mistake. But it would be really nice if we had a way to
flag the need to play with the linker script. Hence perhaps
on new enough tool chains we indeed may want to make use of
--orphan-handling= ? And then discard just .note and .note.*
here?
Jan
On 13/05/2020 10:13, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>
> On 12.05.2020 21:11, Andrew Cooper wrote:
>> @@ -47,6 +47,14 @@ SECTIONS
>> *
>> * Please check build32.mk for more details.
>> */
>> - /* *(.got.plt) */
>> + *(.got.plt)
>> + }
>> +
>> + /DISCARD/ : {
>> + /*
>> + * Discard everything else, to prevent linkers from putting
>> + * orphaned sections ahead of .text, which needs to be first.
>> + */
>> + *(*)
>> }
>> }
> To be honest I'm not sure if this isn't going too far. Much
> depends on what would happen if a new real section appeared
> that needs retaining.
Anything which is important enough will result in a linker error.
> Granted the linker may then once again
> put it at the beginning of the image, and we'll be screwed
> again, just like we'll be screwed if a section gets discarded
> by mistake.
This way is more likely to result in a build failure than an inability
to boot the resulting build of Xen.
> But it would be really nice if we had a way to
> flag the need to play with the linker script. Hence perhaps
> on new enough tool chains we indeed may want to make use of
> --orphan-handling= ? And then discard just .note and .note.*
> here?
The only valid option would be =error, but experimenting with that yields
ld: error: unplaced orphan section `.comment' from `cmdline.o'
ld: error: unplaced orphan section `.note.GNU-stack' from `cmdline.o'
ld: error: unplaced orphan section `.note.gnu.property' from `cmdline.o'
ld: error: unplaced orphan section `.rel.got' from `cmdline.o'
ld: error: unplaced orphan section `.got' from `cmdline.o'
ld: error: unplaced orphan section `.got.plt' from `cmdline.o'
ld: error: unplaced orphan section `.iplt' from `cmdline.o'
ld: error: unplaced orphan section `.rel.iplt' from `cmdline.o'
ld: error: unplaced orphan section `.igot.plt' from `cmdline.o'
which I think is going to get us massively bogged down in toolchain
specifics. I'm not entirely convinced this would be a good move.
~Andrew
On 13.05.2020 17:00, Andrew Cooper wrote:
> On 13/05/2020 10:13, Jan Beulich wrote:
>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe.
>>
>> On 12.05.2020 21:11, Andrew Cooper wrote:
>>> @@ -47,6 +47,14 @@ SECTIONS
>>> *
>>> * Please check build32.mk for more details.
>>> */
>>> - /* *(.got.plt) */
>>> + *(.got.plt)
>>> + }
>>> +
>>> + /DISCARD/ : {
>>> + /*
>>> + * Discard everything else, to prevent linkers from putting
>>> + * orphaned sections ahead of .text, which needs to be first.
>>> + */
>>> + *(*)
>>> }
>>> }
>> To be honest I'm not sure if this isn't going too far. Much
>> depends on what would happen if a new real section appeared
>> that needs retaining.
>
> Anything which is important enough will result in a linker error.
>
>> Granted the linker may then once again
>> put it at the beginning of the image, and we'll be screwed
>> again, just like we'll be screwed if a section gets discarded
>> by mistake.
>
> This way is more likely to result in a build failure than an inability
> to boot the resulting build of Xen.
>
>> But it would be really nice if we had a way to
>> flag the need to play with the linker script. Hence perhaps
>> on new enough tool chains we indeed may want to make use of
>> --orphan-handling= ? And then discard just .note and .note.*
>> here?
>
> The only valid option would be =error, but experimenting with that yields
>
> ld: error: unplaced orphan section `.comment' from `cmdline.o'
> ld: error: unplaced orphan section `.note.GNU-stack' from `cmdline.o'
> ld: error: unplaced orphan section `.note.gnu.property' from `cmdline.o'
> ld: error: unplaced orphan section `.rel.got' from `cmdline.o'
> ld: error: unplaced orphan section `.got' from `cmdline.o'
> ld: error: unplaced orphan section `.got.plt' from `cmdline.o'
> ld: error: unplaced orphan section `.iplt' from `cmdline.o'
> ld: error: unplaced orphan section `.rel.iplt' from `cmdline.o'
> ld: error: unplaced orphan section `.igot.plt' from `cmdline.o'
>
> which I think is going to get us massively bogged down in toolchain
> specifics. I'm not entirely convinced this would be a good move.
That's ugly indeed; especially the .rel.* sections are worrying to
appear there. Hence patch as is
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
On 13/05/2020 16:15, Jan Beulich wrote: >>> But it would be really nice if we had a way to >>> flag the need to play with the linker script. Hence perhaps >>> on new enough tool chains we indeed may want to make use of >>> --orphan-handling= ? And then discard just .note and .note.* >>> here? >> The only valid option would be =error, but experimenting with that yields >> >> ld: error: unplaced orphan section `.comment' from `cmdline.o' >> ld: error: unplaced orphan section `.note.GNU-stack' from `cmdline.o' >> ld: error: unplaced orphan section `.note.gnu.property' from `cmdline.o' >> ld: error: unplaced orphan section `.rel.got' from `cmdline.o' >> ld: error: unplaced orphan section `.got' from `cmdline.o' >> ld: error: unplaced orphan section `.got.plt' from `cmdline.o' >> ld: error: unplaced orphan section `.iplt' from `cmdline.o' >> ld: error: unplaced orphan section `.rel.iplt' from `cmdline.o' >> ld: error: unplaced orphan section `.igot.plt' from `cmdline.o' >> >> which I think is going to get us massively bogged down in toolchain >> specifics. I'm not entirely convinced this would be a good move. > That's ugly indeed; especially the .rel.* sections are worrying to > appear there. What is even more curious, most of them don't exist in cmdine.o Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .group GROUP 00000000 000034 000008 04 14 38 4 [ 2] .group GROUP 00000000 00003c 000008 04 14 40 4 [ 3] .text PROGBITS 00000000 000044 00094a 00 AX 0 0 1 [ 4] .rel.text REL 00000000 000e88 0001e8 08 I 14 3 4 [ 5] .data PROGBITS 00000000 00098e 000000 00 WA 0 0 1 [ 6] .bss NOBITS 00000000 00098e 000000 00 WA 0 0 1 [ 7] .rodata PROGBITS 00000000 000990 0000f3 00 A 0 0 4 [ 8] .rel.rodata REL 00000000 001070 000120 08 I 14 7 4 [ 9] .text.__x86.get_pc_thunk.ax PROGBITS 00000000 000a83 000004 00 AXG 0 0 1 [10] .text.__x86.get_pc_thunk.bx PROGBITS 00000000 000a87 000004 00 AXG 0 0 1 [11] .comment PROGBITS 00000000 000a8b 00002d 01 MS 0 0 1 [12] .note.GNU-stack PROGBITS 00000000 000ab8 000000 00 0 0 1 [13] .note.gnu.property NOTE 00000000 000ab8 00001c 00 A 0 0 4 [14] .symtab SYMTAB 00000000 000ad4 000290 10 15 36 4 [15] .strtab STRTAB 00000000 000d64 000124 00 0 0 1 [16] .shstrtab STRTAB 00000000 001190 0000a7 00 0 0 1 I suspect they are inserted by default as part of processing the relocations, and end up empty. With =warn rather than =error, we instead get ld: warning: orphan section `.comment' from `cmdline.o' being placed in section `.comment' ld: warning: orphan section `.note.GNU-stack' from `cmdline.o' being placed in section `.note.GNU-stack' ld: warning: orphan section `.note.gnu.property' from `cmdline.o' being placed in section `.note.gnu.property' ld: warning: orphan section `.rel.got' from `cmdline.o' being placed in section `.rel.dyn' ld: warning: orphan section `.got' from `cmdline.o' being placed in section `.got' ld: warning: orphan section `.got.plt' from `cmdline.o' being placed in section `.got.plt' ld: warning: orphan section `.iplt' from `cmdline.o' being placed in section `.iplt' ld: warning: orphan section `.rel.iplt' from `cmdline.o' being placed in section `.rel.dyn' ld: warning: orphan section `.igot.plt' from `cmdline.o' being placed in section `.igot.plt' and Section Headers: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al [ 0] NULL 00000000 000000 000000 00 0 0 0 [ 1] .note.gnu.property NOTE 00000000 0000b4 00001c 00 A 0 0 4 [ 2] .text PROGBITS 0000001c 0000d0 000a47 00 WAX 0 0 4 [ 3] .got.plt PROGBITS 00000a64 000b18 00000c 04 WA 0 0 4 [ 4] .comment PROGBITS 00000000 000b24 00002c 01 MS 0 0 1 [ 5] .symtab SYMTAB 00000000 000b50 000230 10 6 31 4 [ 6] .strtab STRTAB 00000000 000d80 000124 00 0 0 1 [ 7] .shstrtab STRTAB 00000000 000ea4 000046 00 0 0 1 in cmdline.lnk, so the .rel.* sections have been dropped overall. I think the =error logic is simply at an unhelpful position during processing. > Hence patch as is Acked-by: Jan Beulich <jbeulich@suse.com> Thanks, As I say, I have a plan to replace all of this completely when a bit more of kbuild is in place. ~Andrew
© 2016 - 2024 Red Hat, Inc.