kexec breaks with 5.4 due to memzero_explicit

kexec breaks with 5.4 due to memzero_explicit

[Arvind Sankar]

Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
according to git bisect.

The patch mentions that it impacts purgatory code, but I don't see any
changes to actually include the definition of memzero_explicit into
purgatory? It used to get memset from arch/x86/boot/compressed/string.c
I think.

Re: kexec breaks with 5.4 due to memzero_explicit

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

Hi,

On 07-10-2019 05:09, Arvind Sankar wrote:
> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
> according to git bisect.

Hmm, it (obviously) does build for me and using kexec still also works
for me.

But it seems that you are right and that this should not build, weird.

Thank you for reporting this. I've attached a patch which should fix this,
I'm also sending this the regular way, so that the x86 maintainers can pick it up.

Can you please give this a try and let us know if it fixes things for you?

Regards,

Hans

[-- Attachment #2: 0001-x86-boot-Provide-memzero_explicit.patch --]
[-- Type: text/x-patch, Size: 1100 bytes --]

From d371dbdef635b57d993bda428a9eb6b929f4472d Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 7 Oct 2019 10:43:00 +0200
Subject: [PATCH] x86/boot: Provide memzero_explicit

The purgatory code now uses the shared lib/crypto/sha256.c sha256
implementation. This needs memzero_explicit, implement this.

Reported-by: Arvind Sankar <nivedita@alum.mit.edu>
Fixes: 906a4bb97f5d ("crypto: sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/x86/boot/compressed/string.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c
index 81fc1eaa3229..511332e279fe 100644
--- a/arch/x86/boot/compressed/string.c
+++ b/arch/x86/boot/compressed/string.c
@@ -50,6 +50,11 @@ void *memset(void *s, int c, size_t n)
 	return s;
 }
 
+void memzero_explicit(void *s, size_t count)
+{
+	memset(s, 0, count);
+}
+
 void *memmove(void *dest, const void *src, size_t n)
 {
 	unsigned char *d = dest;
-- 
2.23.0

Re: kexec breaks with 5.4 due to memzero_explicit

[Hans de Goede]

Hi,

On 07-10-2019 10:50, Hans de Goede wrote:
> Hi,
> 
> On 07-10-2019 05:09, Arvind Sankar wrote:
>> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
>> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
>> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
>> according to git bisect.
> 
> Hmm, it (obviously) does build for me and using kexec still also works
> for me.
> 
> But it seems that you are right and that this should not build, weird.

Ok, I understand now, it seems that the kernel will happily build with
undefined symbols in the purgatory and my kexec testing did not hit
the sha256 check path (*) so it did not crash. I can reproduce this before my patch:

[hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'

And I can confirm that it is gone after my patch:

[hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000

Regards,

Hans


*) I tried with a Fedora signed kernel, dunno how to trigger this if that does not
trigger it

Re: kexec breaks with 5.4 due to memzero_explicit

[Ingo Molnar]

* Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 07-10-2019 10:50, Hans de Goede wrote:
> > Hi,
> > 
> > On 07-10-2019 05:09, Arvind Sankar wrote:
> > > Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
> > > memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
> > > sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
> > > according to git bisect.
> > 
> > Hmm, it (obviously) does build for me and using kexec still also works
> > for me.
> > 
> > But it seems that you are right and that this should not build, weird.
> 
> Ok, I understand now, it seems that the kernel will happily build with
> undefined symbols in the purgatory and my kexec testing did not hit
> the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
> 
> [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
> ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'

I've applied your fix, but would it make sense to also integrate this 
linker test in the regular build with a second patch, to make sure 
something similar doesn't occur again?

Thanks,

	Ingo

Re: kexec breaks with 5.4 due to memzero_explicit

[Arvind Sankar]

On Mon, Oct 07, 2019 at 11:10:18AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07-10-2019 10:50, Hans de Goede wrote:
> > Hi,
> > 
> > On 07-10-2019 05:09, Arvind Sankar wrote:
> >> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
> >> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
> >> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
> >> according to git bisect.
> > 
> > Hmm, it (obviously) does build for me and using kexec still also works
> > for me.
> > 
> > But it seems that you are right and that this should not build, weird.
> 
> Ok, I understand now, it seems that the kernel will happily build with
> undefined symbols in the purgatory and my kexec testing did not hit
> the sha256 check path (*) so it did not crash. I can reproduce this before my patch:

Yes -- this should really be fixed. purgatory build should fail if there
are undefined symbols, in fact the Makefile apparently is trying to do
something to catch undefined references?

LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib

This doesn't seem to actually do anything though. Anyone know of a way
to force ld to error if the resulting object would have undefined
symbols?

> 
> [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
> ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> 
> And I can confirm that it is gone after my patch:
> 
> [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
> ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
> 
> Regards,
> 
> Hans
> 
> 
> *) I tried with a Fedora signed kernel, dunno how to trigger this if that does not
> trigger it
> 

It triggers an error for me when loading the new image, i.e. when doing
# kexec -s -l new_image

Not sure what the difference is, mine is a custom configuration built
using mainline sources.

Re: kexec breaks with 5.4 due to memzero_explicit

[Hans de Goede]

Hi,

On 07-10-2019 15:09, Ingo Molnar wrote:
> 
> * Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Hi,
>>
>> On 07-10-2019 10:50, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-10-2019 05:09, Arvind Sankar wrote:
>>>> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
>>>> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
>>>> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
>>>> according to git bisect.
>>>
>>> Hmm, it (obviously) does build for me and using kexec still also works
>>> for me.
>>>
>>> But it seems that you are right and that this should not build, weird.
>>
>> Ok, I understand now, it seems that the kernel will happily build with
>> undefined symbols in the purgatory and my kexec testing did not hit
>> the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
>>
>> [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
>> ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
>> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
>> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> 
> I've applied your fix,

Thank you, unfortunately I was just minutes away from sending a v2
which adds a missing barrier call (not strictly necessary, more future
proofing).

Hopefully you can still pick up v2 instead, let me know if you want
an incremental patch instead.

Regards,

Hans

Re: kexec breaks with 5.4 due to memzero_explicit

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Hans de Goede <hdegoede@redhat.com> wrote:
> 
> > Hi,
> > 
> > On 07-10-2019 10:50, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 07-10-2019 05:09, Arvind Sankar wrote:
> > > > Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
> > > > memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
> > > > sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
> > > > according to git bisect.
> > > 
> > > Hmm, it (obviously) does build for me and using kexec still also works
> > > for me.
> > > 
> > > But it seems that you are right and that this should not build, weird.
> > 
> > Ok, I understand now, it seems that the kernel will happily build with
> > undefined symbols in the purgatory and my kexec testing did not hit
> > the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
> > 
> > [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
> > ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
> > ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> > sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> 
> I've applied your fix, but would it make sense to also integrate this 
> linker test in the regular build with a second patch, to make sure 
> something similar doesn't occur again?

Note that I delayed the v1 fix and will wait for your v2 fix instead.

Thanks,

	Ingo

Re: kexec breaks with 5.4 due to memzero_explicit

[Ingo Molnar]

* Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 07-10-2019 15:09, Ingo Molnar wrote:
> > 
> > * Hans de Goede <hdegoede@redhat.com> wrote:
> > 
> > > Hi,
> > > 
> > > On 07-10-2019 10:50, Hans de Goede wrote:
> > > > Hi,
> > > > 
> > > > On 07-10-2019 05:09, Arvind Sankar wrote:
> > > > > Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
> > > > > memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
> > > > > sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
> > > > > according to git bisect.
> > > > 
> > > > Hmm, it (obviously) does build for me and using kexec still also works
> > > > for me.
> > > > 
> > > > But it seems that you are right and that this should not build, weird.
> > > 
> > > Ok, I understand now, it seems that the kernel will happily build with
> > > undefined symbols in the purgatory and my kexec testing did not hit
> > > the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
> > > 
> > > [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
> > > ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
> > > ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
> > > sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> > 
> > I've applied your fix,
> 
> Thank you, unfortunately I was just minutes away from sending a v2
> which adds a missing barrier call (not strictly necessary, more future
> proofing).
> 
> Hopefully you can still pick up v2 instead, let me know if you want
> an incremental patch instead.

Yeah, our mails crossed: I noticed that and didn't push out your fix, so 
all should be good. Take your time.

Thanks,

	Ingo

Re: kexec breaks with 5.4 due to memzero_explicit

[Hans de Goede]

Hi,

On 07-10-2019 15:09, Ingo Molnar wrote:
> 
> * Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Hi,
>>
>> On 07-10-2019 10:50, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-10-2019 05:09, Arvind Sankar wrote:
>>>> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
>>>> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
>>>> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
>>>> according to git bisect.
>>>
>>> Hmm, it (obviously) does build for me and using kexec still also works
>>> for me.
>>>
>>> But it seems that you are right and that this should not build, weird.
>>
>> Ok, I understand now, it seems that the kernel will happily build with
>> undefined symbols in the purgatory and my kexec testing did not hit
>> the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
>>
>> [hans@shalem linux]$ ld arch/x86/purgatory/purgatory.ro
>> ld: warning: cannot find entry symbol _start; defaulting to 0000000000401000
>> ld: arch/x86/purgatory/purgatory.ro: in function `sha256_transform':
>> sha256.c:(.text+0x1c0c): undefined reference to `memzero_explicit'
> 
> I've applied your fix, 

I already answered this bit.

> but would it make sense to also integrate this
> linker test in the regular build with a second patch, to make sure
> something similar doesn't occur again?

But I forgot to answer this part, yes I will look into making the build
fail as soon as we have the fix for this in place for 5.4 .

Regards,

Hans

Re: kexec breaks with 5.4 due to memzero_explicit

[Hans de Goede]

Hi,

On 07-10-2019 15:20, Arvind Sankar wrote:
> On Mon, Oct 07, 2019 at 11:10:18AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 07-10-2019 10:50, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 07-10-2019 05:09, Arvind Sankar wrote:
>>>> Hi, arch/x86/purgatory/purgatory.ro has an undefined symbol
>>>> memzero_explicit. This has come from commit 906a4bb97f5d ("crypto:
>>>> sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit")
>>>> according to git bisect.
>>>
>>> Hmm, it (obviously) does build for me and using kexec still also works
>>> for me.
>>>
>>> But it seems that you are right and that this should not build, weird.
>>
>> Ok, I understand now, it seems that the kernel will happily build with
>> undefined symbols in the purgatory and my kexec testing did not hit
>> the sha256 check path (*) so it did not crash. I can reproduce this before my patch:
> 
> Yes -- this should really be fixed. purgatory build should fail if there
> are undefined symbols, in fact the Makefile apparently is trying to do
> something to catch undefined references?
> 
> LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
> 
> This doesn't seem to actually do anything though. Anyone know of a way
> to force ld to error if the resulting object would have undefined
> symbols?

I've figured out a way to get an error for the missing symbol, I will
Cc you on the patch which I will post upstream soon.

I will also write a similar patch for s390 and post that upstream
(untested) separately.

Regards,

Hans