[PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

All of the sudden ld creates base relocations itself, for PE
executables - as a result we now have two of them for every entity to
be relocated. While we will likely want to use this down the road, it
doesn't work quite right yet in corner cases, so rather than suppressing
our own way of creating the relocations we need to tell ld to avoid
doing so.

Probe whether --disable-reloc-section (which was introduced by the same
commit making relocation generation the default) is recognized by ld's PE
emulation, and use the option if so. (To limit redundancy, move the first
part of setting EFI_LDFLAGS earlier, and use it already while probing.)

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -123,8 +123,13 @@ ifneq ($(efi-y),)
 # Check if the compiler supports the MS ABI.
 export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
 # Check if the linker supports PE.
-XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
+EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
+XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
 CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
+# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
+XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
+                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
+                                 echo --disable-reloc-section))
 endif
 
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
@@ -177,8 +182,7 @@ note.o: $(TARGET)-syms
 		--rename-section=.data=.note.gnu.build-id -S $@.bin $@
 	rm -f $@.bin
 
-EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10
-EFI_LDFLAGS += --image-base=$(1) --stack=0,0 --heap=0,0 --strip-debug
+EFI_LDFLAGS += --image-base=$(1) --stack=0,0 --heap=0,0 $(XEN_NO_PE_FIXUPS)
 EFI_LDFLAGS += --section-alignment=0x200000 --file-alignment=0x20
 EFI_LDFLAGS += --major-image-version=$(XEN_VERSION)
 EFI_LDFLAGS += --minor-image-version=$(XEN_SUBVERSION)

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Andrew Cooper]

On 19/02/2021 08:09, Jan Beulich wrote:
> All of the sudden ld creates base relocations itself, for PE
> executables - as a result we now have two of them for every entity to
> be relocated. While we will likely want to use this down the road, it
> doesn't work quite right yet in corner cases, so rather than suppressing
> our own way of creating the relocations we need to tell ld to avoid
> doing so.
>
> Probe whether --disable-reloc-section (which was introduced by the same
> commit making relocation generation the default) is recognized by ld's PE
> emulation, and use the option if so. (To limit redundancy, move the first
> part of setting EFI_LDFLAGS earlier, and use it already while probing.)
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>  # Check if the compiler supports the MS ABI.
>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>  # Check if the linker supports PE.
> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
> +                                 echo --disable-reloc-section))

Why does --strip-debug move?

What's wrong with $(call ld-option ...) ?  Actually, lots of this block
of code looks to be opencoding of standard constructs.

~Andrew

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

On 22.02.2021 17:36, Andrew Cooper wrote:
> On 19/02/2021 08:09, Jan Beulich wrote:
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>  # Check if the compiler supports the MS ABI.
>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>>  # Check if the linker supports PE.
>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>> +                                 echo --disable-reloc-section))
> 
> Why does --strip-debug move?

-S and --strip-debug are the same. I'm simply accumulating in
EFI_LDFLAGS all that's needed for the use in the probing construct.

Also I meanwhile have a patch to retain debug info, for which this
movement turns out to be a prereq. (I've yet to test that the
produced binary actually works, and what's more I first need to get
a couple of changes accepted into binutils for the linker to actually
cope.)

> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
> of code looks to be opencoding of standard constructs.

It looks like ld-option could indeed be used here (there are marginal
differences which are likely acceptable), despite its brief comment
talking of just "flag" (singular, plus not really covering e.g. input
files).

But:
- It working differently than cc-option makes it inconsistent to
  use (the setting of XEN_BUILD_EFI can't very well be switched to
  use cc-option); because of this I'm not surprised that we have
  only exactly one use right now in the tree.
- While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
  another transformation would then be necessary to translate "y"
  into "--disable-reloc-section".
- Do you really suggest to re-do this at this point in the release
  cycle?

Jan

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Andrew Cooper]

On 23/02/2021 07:53, Jan Beulich wrote:
> On 22.02.2021 17:36, Andrew Cooper wrote:
>> On 19/02/2021 08:09, Jan Beulich wrote:
>>> --- a/xen/arch/x86/Makefile
>>> +++ b/xen/arch/x86/Makefile
>>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>>  # Check if the compiler supports the MS ABI.
>>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>>>  # Check if the linker supports PE.
>>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
>>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
>>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>>> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>>> +                                 echo --disable-reloc-section))
>> Why does --strip-debug move?
> -S and --strip-debug are the same. I'm simply accumulating in
> EFI_LDFLAGS all that's needed for the use in the probing construct.

Oh ok.

It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
block, but appended to later on while unprotected.  That said, I'm
fairly sure it is only consumed inside a different ifeq section, so I
think there is a reasonable quantity of tidying which ought to be done here.

> Also I meanwhile have a patch to retain debug info, for which this
> movement turns out to be a prereq. (I've yet to test that the
> produced binary actually works, and what's more I first need to get
> a couple of changes accepted into binutils for the linker to actually
> cope.)
>
>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>> of code looks to be opencoding of standard constructs.
> It looks like ld-option could indeed be used here (there are marginal
> differences which are likely acceptable), despite its brief comment
> talking of just "flag" (singular, plus not really covering e.g. input
> files).
>
> But:
> - It working differently than cc-option makes it inconsistent to
>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>   use cc-option); because of this I'm not surprised that we have
>   only exactly one use right now in the tree.
> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>   another transformation would then be necessary to translate "y"
>   into "--disable-reloc-section".
> - Do you really suggest to re-do this at this point in the release
>   cycle?

I'm looking to prevent this almost-incompressible mess from getting worse.

But I suppose you want this to backport, so I suppose it ought to be
minimally invasive.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

This logic all actually needs moving into Kconfig so we can also go
about fixing the other bugs we have such as having Multiboot headers in
xen.efi pointing at unusable entrypoints.

~Andrew

Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

On 24.02.2021 18:17, Andrew Cooper wrote:
> On 23/02/2021 07:53, Jan Beulich wrote:
>> On 22.02.2021 17:36, Andrew Cooper wrote:
>>> On 19/02/2021 08:09, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/Makefile
>>>> +++ b/xen/arch/x86/Makefile
>>>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>>>  # Check if the compiler supports the MS ABI.
>>>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>>>>  # Check if the linker supports PE.
>>>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
>>>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>>> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
>>>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>>>> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>>>> +                                 echo --disable-reloc-section))
>>> Why does --strip-debug move?
>> -S and --strip-debug are the same. I'm simply accumulating in
>> EFI_LDFLAGS all that's needed for the use in the probing construct.
> 
> Oh ok.
> 
> It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
> block, but appended to later on while unprotected.  That said, I'm
> fairly sure it is only consumed inside a different ifeq section, so I
> think there is a reasonable quantity of tidying which ought to be done here.

Yes, in particular it wants to move out of Makefile (so it
won't get executed multiple times).

>> Also I meanwhile have a patch to retain debug info, for which this
>> movement turns out to be a prereq. (I've yet to test that the
>> produced binary actually works, and what's more I first need to get
>> a couple of changes accepted into binutils for the linker to actually
>> cope.)
>>
>>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>>> of code looks to be opencoding of standard constructs.
>> It looks like ld-option could indeed be used here (there are marginal
>> differences which are likely acceptable), despite its brief comment
>> talking of just "flag" (singular, plus not really covering e.g. input
>> files).
>>
>> But:
>> - It working differently than cc-option makes it inconsistent to
>>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>>   use cc-option); because of this I'm not surprised that we have
>>   only exactly one use right now in the tree.
>> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>>   another transformation would then be necessary to translate "y"
>>   into "--disable-reloc-section".
>> - Do you really suggest to re-do this at this point in the release
>>   cycle?
> 
> I'm looking to prevent this almost-incompressible mess from getting worse.
> 
> But I suppose you want this to backport, so I suppose it ought to be
> minimally invasive.

Backporting - yes, definitely. And hence minimally invasive would
indeed be helpful.

> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks.

> This logic all actually needs moving into Kconfig so we can also go
> about fixing the other bugs we have such as having Multiboot headers in
> xen.efi pointing at unusable entrypoints.

My objections to doing such stuff in Kconfig have remained
unresponded to, iirc. Plus doing this in Kconfig would help on its
own - we'd also need to further split which object files get linked
into which binary. (In fact a patch in the 4.16 series I have now
to use linker produced base relocations and to retain debug info I
do away with prelink-efi.o, for becoming identical to prelink.o.)

Jan

[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Jan Beulich]

On 24.02.2021 18:17, Andrew Cooper wrote:
> On 23/02/2021 07:53, Jan Beulich wrote:
>> On 22.02.2021 17:36, Andrew Cooper wrote:
>>> On 19/02/2021 08:09, Jan Beulich wrote:
>>>> --- a/xen/arch/x86/Makefile
>>>> +++ b/xen/arch/x86/Makefile
>>>> @@ -123,8 +123,13 @@ ifneq ($(efi-y),)
>>>>  # Check if the compiler supports the MS ABI.
>>>>  export XEN_BUILD_EFI := $(shell $(CC) $(XEN_CFLAGS) -c efi/check.c -o efi/check.o 2>/dev/null && echo y)
>>>>  # Check if the linker supports PE.
>>>> -XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) -mi386pep --subsystem=10 -S -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>> +EFI_LDFLAGS = $(patsubst -m%,-mi386pep,$(XEN_LDFLAGS)) --subsystem=10 --strip-debug
>>>> +XEN_BUILD_PE := $(if $(XEN_BUILD_EFI),$(shell $(LD) $(EFI_LDFLAGS) -o efi/check.efi efi/check.o 2>/dev/null && echo y))
>>>>  CFLAGS-$(XEN_BUILD_EFI) += -DXEN_BUILD_EFI
>>>> +# Check if the linker produces fixups in PE by default (we need to disable it doing so for now).
>>>> +XEN_NO_PE_FIXUPS := $(if $(XEN_BUILD_EFI), \
>>>> +                         $(shell $(LD) $(EFI_LDFLAGS) --disable-reloc-section -o efi/check.efi efi/check.o 2>/dev/null && \
>>>> +                                 echo --disable-reloc-section))
>>> Why does --strip-debug move?
>> -S and --strip-debug are the same. I'm simply accumulating in
>> EFI_LDFLAGS all that's needed for the use in the probing construct.
> 
> Oh ok.
> 
> It occurs to me that EFI_LDFLAGS now only gets started in an ifneq
> block, but appended to later on while unprotected.  That said, I'm
> fairly sure it is only consumed inside a different ifeq section, so I
> think there is a reasonable quantity of tidying which ought to be done here.
> 
>> Also I meanwhile have a patch to retain debug info, for which this
>> movement turns out to be a prereq. (I've yet to test that the
>> produced binary actually works, and what's more I first need to get
>> a couple of changes accepted into binutils for the linker to actually
>> cope.)
>>
>>> What's wrong with $(call ld-option ...) ?  Actually, lots of this block
>>> of code looks to be opencoding of standard constructs.
>> It looks like ld-option could indeed be used here (there are marginal
>> differences which are likely acceptable), despite its brief comment
>> talking of just "flag" (singular, plus not really covering e.g. input
>> files).
>>
>> But:
>> - It working differently than cc-option makes it inconsistent to
>>   use (the setting of XEN_BUILD_EFI can't very well be switched to
>>   use cc-option); because of this I'm not surprised that we have
>>   only exactly one use right now in the tree.
>> - While XEN_BUILD_PE wants to be set to "y", for XEN_NO_PE_FIXUPS
>>   another transformation would then be necessary to translate "y"
>>   into "--disable-reloc-section".
>> - Do you really suggest to re-do this at this point in the release
>>   cycle?
> 
> I'm looking to prevent this almost-incompressible mess from getting worse.
> 
> But I suppose you want this to backport, so I suppose it ought to be
> minimally invasive.
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Since getting Andrew's ack has taken across the firm-freeze boundary,
may I ask for a release-ack here? As noted this change (alongside
the earlier one) will want backporting, perhaps even to security-
support-only branches.

Jan

[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs

[Ian Jackson]

Jan Beulich writes ("[4.15] Re: [PATCH] x86/EFI: suppress GNU ld 2.36'es creation of base relocs"):
> Since getting Andrew's ack has taken across the firm-freeze boundary,
> may I ask for a release-ack here?

Indeed.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

>  As noted this change (alongside
> the earlier one) will want backporting, perhaps even to security-
> support-only branches.

Noted.

Ian.