x86/purgatory: Make sure we fail the build if purgatory.ro has missing symbols
diff mbox series

Message ID 20191007175546.3395-1-hdegoede@redhat.com
State New
Headers show
Series
  • x86/purgatory: Make sure we fail the build if purgatory.ro has missing symbols
Related show

Commit Message

Hans de Goede Oct. 7, 2019, 5:55 p.m. UTC
Since we link purgatory.ro with -r aka we enable "incremental linking"
no checks for unresolved symbols is done while linking purgatory.ro.

Changes to the sha256 code has caused the purgatory in 5.4-rc1 to have
a missing symbol on memzero_explicit, yet things still happily build.

This commit adds an extra check for unresolved symbols by calling ld
without -r before running bin2c to generate kexec-purgatory.c.

This causes a build of 5.4-rc1 with this patch added to fail as it should:

  CHK     arch/x86/purgatory/purgatory.ro
ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
make[2]: *** [arch/x86/purgatory/Makefile:72:
    arch/x86/purgatory/kexec-purgatory.c] Error 1
make[1]: *** [scripts/Makefile.build:509: arch/x86/purgatory] Error 2
make: *** [Makefile:1650: arch/x86] Error 2

This will help us catch missing symbols in the purgatory sooner.

Note this commit also removes --no-undefined from LDFLAGS_purgatory.ro
as that has no effect.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/x86/purgatory/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Nathan Chancellor Oct. 7, 2019, 8:05 p.m. UTC | #1
On Mon, Oct 07, 2019 at 07:55:46PM +0200, Hans de Goede wrote:
> Since we link purgatory.ro with -r aka we enable "incremental linking"
> no checks for unresolved symbols is done while linking purgatory.ro.
> 
> Changes to the sha256 code has caused the purgatory in 5.4-rc1 to have
> a missing symbol on memzero_explicit, yet things still happily build.
> 
> This commit adds an extra check for unresolved symbols by calling ld
> without -r before running bin2c to generate kexec-purgatory.c.
> 
> This causes a build of 5.4-rc1 with this patch added to fail as it should:
> 
>   CHK     arch/x86/purgatory/purgatory.ro
> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> make[2]: *** [arch/x86/purgatory/Makefile:72:
>     arch/x86/purgatory/kexec-purgatory.c] Error 1
> make[1]: *** [scripts/Makefile.build:509: arch/x86/purgatory] Error 2
> make: *** [Makefile:1650: arch/x86] Error 2
> 
> This will help us catch missing symbols in the purgatory sooner.
> 
> Note this commit also removes --no-undefined from LDFLAGS_purgatory.ro
> as that has no effect.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  arch/x86/purgatory/Makefile | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index fb4ee5444379..0da0794ef1f0 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -14,7 +14,7 @@ $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
>  
>  CFLAGS_sha256.o := -D__DISABLE_EXPORTS
>  
> -LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
> +LDFLAGS_purgatory.ro := -e purgatory_start -r -nostdlib -z nodefaultlib
>  targets += purgatory.ro
>  
>  KASAN_SANITIZE	:= n
> @@ -60,10 +60,16 @@ $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
>  
>  targets += kexec-purgatory.c
>  
> +# Since we link purgatory.ro with -r unresolved symbols are not checked,
> +# so we check this before generating kexec-purgatory.c instead
> +quiet_cmd_check_purgatory = CHK     $<
> +      cmd_check_purgatory = ld -e purgatory_start $<

I think this should be $(LD) -e ... so that using a cross compile prefix
(like x86_64-linux-) or an alternative linker like ld.lld works properly.

> +
>  quiet_cmd_bin2c = BIN2C   $@
>        cmd_bin2c = $(objtree)/scripts/bin2c kexec_purgatory < $< > $@
>  
>  $(obj)/kexec-purgatory.c: $(obj)/purgatory.ro FORCE
> +	$(call if_changed,check_purgatory)
>  	$(call if_changed,bin2c)
>  
>  obj-$(CONFIG_KEXEC_FILE)	+= kexec-purgatory.o
> -- 
> 2.23.0
> 

Cheers,
Nathan
Hans de Goede Oct. 7, 2019, 8:31 p.m. UTC | #2
HI,

On 07-10-2019 22:05, Nathan Chancellor wrote:
> On Mon, Oct 07, 2019 at 07:55:46PM +0200, Hans de Goede wrote:
>> Since we link purgatory.ro with -r aka we enable "incremental linking"
>> no checks for unresolved symbols is done while linking purgatory.ro.
>>
>> Changes to the sha256 code has caused the purgatory in 5.4-rc1 to have
>> a missing symbol on memzero_explicit, yet things still happily build.
>>
>> This commit adds an extra check for unresolved symbols by calling ld
>> without -r before running bin2c to generate kexec-purgatory.c.
>>
>> This causes a build of 5.4-rc1 with this patch added to fail as it should:
>>
>>    CHK     arch/x86/purgatory/purgatory.ro
>> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
>> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
>> make[2]: *** [arch/x86/purgatory/Makefile:72:
>>      arch/x86/purgatory/kexec-purgatory.c] Error 1
>> make[1]: *** [scripts/Makefile.build:509: arch/x86/purgatory] Error 2
>> make: *** [Makefile:1650: arch/x86] Error 2
>>
>> This will help us catch missing symbols in the purgatory sooner.
>>
>> Note this commit also removes --no-undefined from LDFLAGS_purgatory.ro
>> as that has no effect.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   arch/x86/purgatory/Makefile | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
>> index fb4ee5444379..0da0794ef1f0 100644
>> --- a/arch/x86/purgatory/Makefile
>> +++ b/arch/x86/purgatory/Makefile
>> @@ -14,7 +14,7 @@ $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
>>   
>>   CFLAGS_sha256.o := -D__DISABLE_EXPORTS
>>   
>> -LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
>> +LDFLAGS_purgatory.ro := -e purgatory_start -r -nostdlib -z nodefaultlib
>>   targets += purgatory.ro
>>   
>>   KASAN_SANITIZE	:= n
>> @@ -60,10 +60,16 @@ $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
>>   
>>   targets += kexec-purgatory.c
>>   
>> +# Since we link purgatory.ro with -r unresolved symbols are not checked,
>> +# so we check this before generating kexec-purgatory.c instead
>> +quiet_cmd_check_purgatory = CHK     $<
>> +      cmd_check_purgatory = ld -e purgatory_start $<
> 
> I think this should be $(LD) -e ... so that using a cross compile prefix
> (like x86_64-linux-) or an alternative linker like ld.lld works properly.

Good point, also the ld command is actually outputting an a.out file
which is also something which we do not want.

I will prepare a new version fixing both.

Regards,

Hans
Arvind Sankar Oct. 7, 2019, 9:52 p.m. UTC | #3
On Mon, Oct 07, 2019 at 10:31:49PM +0200, Hans de Goede wrote:
> HI,
> 
> On 07-10-2019 22:05, Nathan Chancellor wrote:
> > On Mon, Oct 07, 2019 at 07:55:46PM +0200, Hans de Goede wrote:
> >> Since we link purgatory.ro with -r aka we enable "incremental linking"
> >> no checks for unresolved symbols is done while linking purgatory.ro.
> >>
> >> Changes to the sha256 code has caused the purgatory in 5.4-rc1 to have
> >> a missing symbol on memzero_explicit, yet things still happily build.
> >>
> >> This commit adds an extra check for unresolved symbols by calling ld
> >> without -r before running bin2c to generate kexec-purgatory.c.
> >>
> >> This causes a build of 5.4-rc1 with this patch added to fail as it should:
> >>
> >>    CHK     arch/x86/purgatory/purgatory.ro
> >> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> >> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> >> make[2]: *** [arch/x86/purgatory/Makefile:72:
> >>      arch/x86/purgatory/kexec-purgatory.c] Error 1
> >> make[1]: *** [scripts/Makefile.build:509: arch/x86/purgatory] Error 2
> >> make: *** [Makefile:1650: arch/x86] Error 2
> >>
> >> This will help us catch missing symbols in the purgatory sooner.
> >>
> >> Note this commit also removes --no-undefined from LDFLAGS_purgatory.ro
> >> as that has no effect.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> ---
> >>   arch/x86/purgatory/Makefile | 8 +++++++-
> >>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> >> index fb4ee5444379..0da0794ef1f0 100644
> >> --- a/arch/x86/purgatory/Makefile
> >> +++ b/arch/x86/purgatory/Makefile
> >> @@ -14,7 +14,7 @@ $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
> >>   
> >>   CFLAGS_sha256.o := -D__DISABLE_EXPORTS
> >>   
> >> -LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
> >> +LDFLAGS_purgatory.ro := -e purgatory_start -r -nostdlib -z nodefaultlib
> >>   targets += purgatory.ro
> >>   
> >>   KASAN_SANITIZE	:= n
> >> @@ -60,10 +60,16 @@ $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
> >>   
> >>   targets += kexec-purgatory.c
> >>   
> >> +# Since we link purgatory.ro with -r unresolved symbols are not checked,
> >> +# so we check this before generating kexec-purgatory.c instead
> >> +quiet_cmd_check_purgatory = CHK     $<
> >> +      cmd_check_purgatory = ld -e purgatory_start $<
> > 
> > I think this should be $(LD) -e ... so that using a cross compile prefix
> > (like x86_64-linux-) or an alternative linker like ld.lld works properly.
> 
> Good point, also the ld command is actually outputting an a.out file
> which is also something which we do not want.
> 
> I will prepare a new version fixing both.
> 
> Regards,
> 
> Hans

We could just use $(NM) -u, right?
Hans de Goede Oct. 8, 2019, 5:57 a.m. UTC | #4
HI,

On 07-10-2019 23:52, Arvind Sankar wrote:
> On Mon, Oct 07, 2019 at 10:31:49PM +0200, Hans de Goede wrote:
>> HI,
>>
>> On 07-10-2019 22:05, Nathan Chancellor wrote:
>>> On Mon, Oct 07, 2019 at 07:55:46PM +0200, Hans de Goede wrote:
>>>> Since we link purgatory.ro with -r aka we enable "incremental linking"
>>>> no checks for unresolved symbols is done while linking purgatory.ro.
>>>>
>>>> Changes to the sha256 code has caused the purgatory in 5.4-rc1 to have
>>>> a missing symbol on memzero_explicit, yet things still happily build.
>>>>
>>>> This commit adds an extra check for unresolved symbols by calling ld
>>>> without -r before running bin2c to generate kexec-purgatory.c.
>>>>
>>>> This causes a build of 5.4-rc1 with this patch added to fail as it should:
>>>>
>>>>     CHK     arch/x86/purgatory/purgatory.ro
>>>> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
>>>> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
>>>> make[2]: *** [arch/x86/purgatory/Makefile:72:
>>>>       arch/x86/purgatory/kexec-purgatory.c] Error 1
>>>> make[1]: *** [scripts/Makefile.build:509: arch/x86/purgatory] Error 2
>>>> make: *** [Makefile:1650: arch/x86] Error 2
>>>>
>>>> This will help us catch missing symbols in the purgatory sooner.
>>>>
>>>> Note this commit also removes --no-undefined from LDFLAGS_purgatory.ro
>>>> as that has no effect.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>>    arch/x86/purgatory/Makefile | 8 +++++++-
>>>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
>>>> index fb4ee5444379..0da0794ef1f0 100644
>>>> --- a/arch/x86/purgatory/Makefile
>>>> +++ b/arch/x86/purgatory/Makefile
>>>> @@ -14,7 +14,7 @@ $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
>>>>    
>>>>    CFLAGS_sha256.o := -D__DISABLE_EXPORTS
>>>>    
>>>> -LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
>>>> +LDFLAGS_purgatory.ro := -e purgatory_start -r -nostdlib -z nodefaultlib
>>>>    targets += purgatory.ro
>>>>    
>>>>    KASAN_SANITIZE	:= n
>>>> @@ -60,10 +60,16 @@ $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
>>>>    
>>>>    targets += kexec-purgatory.c
>>>>    
>>>> +# Since we link purgatory.ro with -r unresolved symbols are not checked,
>>>> +# so we check this before generating kexec-purgatory.c instead
>>>> +quiet_cmd_check_purgatory = CHK     $<
>>>> +      cmd_check_purgatory = ld -e purgatory_start $<
>>>
>>> I think this should be $(LD) -e ... so that using a cross compile prefix
>>> (like x86_64-linux-) or an alternative linker like ld.lld works properly.
>>
>> Good point, also the ld command is actually outputting an a.out file
>> which is also something which we do not want.
>>
>> I will prepare a new version fixing both.
>>
>> Regards,
>>
>> Hans
> 
> We could just use $(NM) -u, right?

That would require wrapping it in some shell code like this (untested):

SHOULD_BE_EMPTY=$(nm -u purgatory.ro); if [ "$SHOULD_BE_EMPTY" == "" ]; then true; else false; fi

And then escaping the $ in there and in injecting $(NM) into it, IMHO
it is easier to just do:

$(LD) $PURGATORY_LDFLAGS) purgatory.ro -o /dev/null

Also it seems better to actually use the linker for this test then usig an other
tool which may have subtly different semantics.

Regards,

Hans

Patch
diff mbox series

diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index fb4ee5444379..0da0794ef1f0 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -14,7 +14,7 @@  $(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
 
 CFLAGS_sha256.o := -D__DISABLE_EXPORTS
 
-LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
+LDFLAGS_purgatory.ro := -e purgatory_start -r -nostdlib -z nodefaultlib
 targets += purgatory.ro
 
 KASAN_SANITIZE	:= n
@@ -60,10 +60,16 @@  $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
 
 targets += kexec-purgatory.c
 
+# Since we link purgatory.ro with -r unresolved symbols are not checked,
+# so we check this before generating kexec-purgatory.c instead
+quiet_cmd_check_purgatory = CHK     $<
+      cmd_check_purgatory = ld -e purgatory_start $<
+
 quiet_cmd_bin2c = BIN2C   $@
       cmd_bin2c = $(objtree)/scripts/bin2c kexec_purgatory < $< > $@
 
 $(obj)/kexec-purgatory.c: $(obj)/purgatory.ro FORCE
+	$(call if_changed,check_purgatory)
 	$(call if_changed,bin2c)
 
 obj-$(CONFIG_KEXEC_FILE)	+= kexec-purgatory.o