linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
       [not found] ` <20250502011246.99238-2-coxu@redhat.com>
@ 2025-08-21 11:15   ` Breno Leitao
  2025-08-25  1:18     ` Coiby Xu
  0 siblings, 1 reply; 5+ messages in thread
From: Breno Leitao @ 2025-08-21 11:15 UTC (permalink / raw)
  To: Coiby Xu
  Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Arnaud Lefebvre, Jan Pazdziora, Andrew Morton, linux-arm-kernel

Hello Coiby,

On Fri, May 02, 2025 at 09:12:35AM +0800, Coiby Xu wrote:
> +static inline void kexec_random_range_start(unsigned long start,
> +					    unsigned long end,
> +					    struct kexec_buf *kbuf,
> +					    unsigned long *temp_start)
> +{
> +	unsigned short i;
> +
> +	if (kbuf->random) {
> +		get_random_bytes(&i, sizeof(unsigned short));
> +		*temp_start = start + (end - start) / USHRT_MAX * i;
> +	}
> +}

On arm64, I am getting the following UBSAN warning when accessing
kbuf->random:

[   32.362428] ------------[ cut here ]------------
[   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
[   32.362649] load of value 252 is not a valid value for type '_Bool'

and line 210 is your `if (kbuf->random)`.

Basically kbuf was not initialized in arm hosts, and probably has
garbage.

I am wondering if we should have something like , while the support for arm64 is
not done:

commit 2608bd8c26b62a9a7cc50106e93d3a1ffb1e1188
Author: Breno Leitao <leitao@debian.org>
Date:   Thu Aug 21 04:11:21 2025 -0700

    Initialize the random field of kbuf to zero in the ARM64 kexec image loader

    Ads an explicit initialization for the random member of the kbuf
    structure within the image_load function in
    arch/arm64/kernel/kexec_image.c. Setting kbuf.random to zero ensures
    a deterministic and clean starting state for the buffer used during
    kernel image loading, avoiding this UBSAN issue later, when kbuf.random
    is read.

            [   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
            [   32.362649] load of value 252 is not a valid value for type '_Bool'

    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index 532d72ea42ee8..287b25e674d76 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
 	kbuf.buf_min = 0;
 	kbuf.buf_max = ULONG_MAX;
 	kbuf.top_down = false;
+	kbuf.random = 0;

 	kbuf.buffer = kernel;
 	kbuf.bufsz = kernel_len;




^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
  2025-08-21 11:15   ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Breno Leitao
@ 2025-08-25  1:18     ` Coiby Xu
  2025-08-26  1:05       ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Coiby Xu @ 2025-08-25  1:18 UTC (permalink / raw)
  To: Breno Leitao
  Cc: kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Arnaud Lefebvre, Jan Pazdziora, Andrew Morton, linux-arm-kernel

On Thu, Aug 21, 2025 at 04:15:53AM -0700, Breno Leitao wrote:
>Hello Coiby,

Hi Breno,

>
>On Fri, May 02, 2025 at 09:12:35AM +0800, Coiby Xu wrote:
>> +static inline void kexec_random_range_start(unsigned long start,
>> +					    unsigned long end,
>> +					    struct kexec_buf *kbuf,
>> +					    unsigned long *temp_start)
>> +{
>> +	unsigned short i;
>> +
>> +	if (kbuf->random) {
>> +		get_random_bytes(&i, sizeof(unsigned short));
>> +		*temp_start = start + (end - start) / USHRT_MAX * i;
>> +	}
>> +}
>
>On arm64, I am getting the following UBSAN warning when accessing
>kbuf->random:
>
>[   32.362428] ------------[ cut here ]------------
>[   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
>[   32.362649] load of value 252 is not a valid value for type '_Bool'
>
>and line 210 is your `if (kbuf->random)`.
>
>Basically kbuf was not initialized in arm hosts, and probably has
>garbage.

Thank for explaining the problem to me!

>
>I am wondering if we should have something like , while the support for arm64 is
>not done:
>
>commit 2608bd8c26b62a9a7cc50106e93d3a1ffb1e1188
>Author: Breno Leitao <leitao@debian.org>
>Date:   Thu Aug 21 04:11:21 2025 -0700
>
>    Initialize the random field of kbuf to zero in the ARM64 kexec image loader
>
>    Ads an explicit initialization for the random member of the kbuf
>    structure within the image_load function in
>    arch/arm64/kernel/kexec_image.c. Setting kbuf.random to zero ensures
>    a deterministic and clean starting state for the buffer used during
>    kernel image loading, avoiding this UBSAN issue later, when kbuf.random
>    is read.
>
>            [   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
>            [   32.362649] load of value 252 is not a valid value for type '_Bool'
>
>    Signed-off-by: Breno Leitao <leitao@debian.org>
>
>diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
>index 532d72ea42ee8..287b25e674d76 100644
>--- a/arch/arm64/kernel/kexec_image.c
>+++ b/arch/arm64/kernel/kexec_image.c
>@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> 	kbuf.buf_min = 0;
> 	kbuf.buf_max = ULONG_MAX;
> 	kbuf.top_down = false;
>+	kbuf.random = 0;
>
> 	kbuf.buffer = kernel;
> 	kbuf.bufsz = kernel_len;
>

And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
randomly")' and then send it to kexec@lists.infradead.org? Thanks!

-- 
Best regards,
Coiby



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
  2025-08-25  1:18     ` Coiby Xu
@ 2025-08-26  1:05       ` Andrew Morton
  2025-08-27 11:38         ` Baoquan He
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2025-08-26  1:05 UTC (permalink / raw)
  To: Coiby Xu
  Cc: Breno Leitao, kexec, Ondrej Kozina, Milan Broz, Thomas Staudt,
	Daniel P . Berrangé, Kairui Song, Pingfan Liu, Baoquan He,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel

On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:

> >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> >index 532d72ea42ee8..287b25e674d76 100644
> >--- a/arch/arm64/kernel/kexec_image.c
> >+++ b/arch/arm64/kernel/kexec_image.c
> >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > 	kbuf.buf_min = 0;
> > 	kbuf.buf_max = ULONG_MAX;
> > 	kbuf.top_down = false;
> >+	kbuf.random = 0;
> >
> > 	kbuf.buffer = kernel;
> > 	kbuf.bufsz = kernel_len;
> >
> 
> And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> randomly")' and then send it to kexec@lists.infradead.org? Thanks!

I turned all this into a regular patch and queued it (see below),
thanks.  No additional actions are needed.

I'm really not liking that code.  I laboriously verified that all
fields of kexec_buf are now initialized, except for `cma'.  Is that a
bug?

This function has a call frequency of about 3x per week.  Can we please
just memset the whole thing so people don't have to worry about this
any more?


From: Breno Leitao <leitao@debian.org>
Subject: kexec/arm64: initialize the random field of kbuf to zero in the image loader
Date: Thu Aug 21 04:11:21 2025 -0700

Add an explicit initialization for the random member of the kbuf structure
within the image_load function in arch/arm64/kernel/kexec_image.c. 
Setting kbuf.random to zero ensures a deterministic and clean starting
state for the buffer used during kernel image loading, avoiding this UBSAN
issue later, when kbuf.random is read.

  [   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
  [   32.362649] load of value 252 is not a valid value for type '_Bool'

Link: https://lkml.kernel.org/r/oninomspajhxp4omtdapxnckxydbk2nzmrix7rggmpukpnzadw@c67o7njgdgm3
Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf randomly
Signed-off-by: Breno Leitao <leitao@debian.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Coiby Xu <coxu@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: Kairui Song <ryncsn@gmail.com>
Cc: Liu Pingfan <kernelfans@gmail.com>
Cc: Milan Broz <gmazyland@gmail.com>
Cc: Ondrej Kozina <okozina@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/arm64/kernel/kexec_image.c |    1 +
 1 file changed, 1 insertion(+)

--- a/arch/arm64/kernel/kexec_image.c~kexec-arm64-initialize-the-random-field-of-kbuf-to-zero-in-the-image-loader
+++ a/arch/arm64/kernel/kexec_image.c
@@ -76,6 +76,7 @@ static void *image_load(struct kimage *i
 	kbuf.buf_min = 0;
 	kbuf.buf_max = ULONG_MAX;
 	kbuf.top_down = false;
+	kbuf.random = 0;
 
 	kbuf.buffer = kernel;
 	kbuf.bufsz = kernel_len;
_



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
  2025-08-26  1:05       ` Andrew Morton
@ 2025-08-27 11:38         ` Baoquan He
  2025-08-27 13:38           ` Breno Leitao
  0 siblings, 1 reply; 5+ messages in thread
From: Baoquan He @ 2025-08-27 11:38 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Coiby Xu, Breno Leitao, kexec, Ondrej Kozina, Milan Broz,
	Thomas Staudt, Daniel P . Berrangé, Kairui Song, Pingfan Liu,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel

On 08/25/25 at 06:05pm, Andrew Morton wrote:
> On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:
> 
> > >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > >index 532d72ea42ee8..287b25e674d76 100644
> > >--- a/arch/arm64/kernel/kexec_image.c
> > >+++ b/arch/arm64/kernel/kexec_image.c
> > >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > > 	kbuf.buf_min = 0;
> > > 	kbuf.buf_max = ULONG_MAX;
> > > 	kbuf.top_down = false;
> > >+	kbuf.random = 0;
> > >
> > > 	kbuf.buffer = kernel;
> > > 	kbuf.bufsz = kernel_len;
> > >
> > 
> > And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> > tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> > randomly")' and then send it to kexec@lists.infradead.org? Thanks!
> 
> I turned all this into a regular patch and queued it (see below),
> thanks.  No additional actions are needed.
> 
> I'm really not liking that code.  I laboriously verified that all
> fields of kexec_buf are now initialized, except for `cma'.  Is that a
> bug?
> 
> This function has a call frequency of about 3x per week.  Can we please
> just memset the whole thing so people don't have to worry about this
> any more?

Yeah, adding these trivial patches to mute XXSAN warning is annoying.
Maybe arm64 can initialize the local variable kbuf like we do in x86_64
as below, to explicitly set the necessary fields when defining.

static void *bzImage64_load(struct kimage *image, char *kernel,
                            unsigned long kernel_len, char *initrd,
                            unsigned long initrd_len, char *cmdline,
                            unsigned long cmdline_len)
{
......
        struct kexec_buf kbuf = { .image = image, .buf_max = ULONG_MAX,
                                  .top_down = true };
        struct kexec_buf pbuf = { .image = image, .buf_min = MIN_PURGATORY_ADDR,
                                  .buf_max = ULONG_MAX, .top_down = true };
.....
}

> 
> 
> From: Breno Leitao <leitao@debian.org>
> Subject: kexec/arm64: initialize the random field of kbuf to zero in the image loader
> Date: Thu Aug 21 04:11:21 2025 -0700
> 
> Add an explicit initialization for the random member of the kbuf structure
> within the image_load function in arch/arm64/kernel/kexec_image.c. 
> Setting kbuf.random to zero ensures a deterministic and clean starting
> state for the buffer used during kernel image loading, avoiding this UBSAN
> issue later, when kbuf.random is read.
> 
>   [   32.362488] UBSAN: invalid-load in ./include/linux/kexec.h:210:10
>   [   32.362649] load of value 252 is not a valid value for type '_Bool'
> 
> Link: https://lkml.kernel.org/r/oninomspajhxp4omtdapxnckxydbk2nzmrix7rggmpukpnzadw@c67o7njgdgm3
> Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf randomly
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Coiby Xu <coxu@redhat.com>
> Cc: "Daniel P. Berrange" <berrange@redhat.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: Kairui Song <ryncsn@gmail.com>
> Cc: Liu Pingfan <kernelfans@gmail.com>
> Cc: Milan Broz <gmazyland@gmail.com>
> Cc: Ondrej Kozina <okozina@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  arch/arm64/kernel/kexec_image.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/arch/arm64/kernel/kexec_image.c~kexec-arm64-initialize-the-random-field-of-kbuf-to-zero-in-the-image-loader
> +++ a/arch/arm64/kernel/kexec_image.c
> @@ -76,6 +76,7 @@ static void *image_load(struct kimage *i
>  	kbuf.buf_min = 0;
>  	kbuf.buf_max = ULONG_MAX;
>  	kbuf.top_down = false;
> +	kbuf.random = 0;
>  
>  	kbuf.buffer = kernel;
>  	kbuf.bufsz = kernel_len;
> _
> 



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly
  2025-08-27 11:38         ` Baoquan He
@ 2025-08-27 13:38           ` Breno Leitao
  0 siblings, 0 replies; 5+ messages in thread
From: Breno Leitao @ 2025-08-27 13:38 UTC (permalink / raw)
  To: Baoquan He
  Cc: Andrew Morton, Coiby Xu, kexec, Ondrej Kozina, Milan Broz,
	Thomas Staudt, Daniel P . Berrangé, Kairui Song, Pingfan Liu,
	Dave Young, linux-kernel, x86, Dave Hansen, Vitaly Kuznetsov,
	Arnaud Lefebvre, Jan Pazdziora, linux-arm-kernel

On Wed, Aug 27, 2025 at 07:38:29PM +0800, Baoquan He wrote:
> On 08/25/25 at 06:05pm, Andrew Morton wrote:
> > On Mon, 25 Aug 2025 09:18:53 +0800 Coiby Xu <coxu@redhat.com> wrote:
> > 
> > > >diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > > >index 532d72ea42ee8..287b25e674d76 100644
> > > >--- a/arch/arm64/kernel/kexec_image.c
> > > >+++ b/arch/arm64/kernel/kexec_image.c
> > > >@@ -76,6 +76,7 @@ static void *image_load(struct kimage *image,
> > > > 	kbuf.buf_min = 0;
> > > > 	kbuf.buf_max = ULONG_MAX;
> > > > 	kbuf.top_down = false;
> > > >+	kbuf.random = 0;
> > > >
> > > > 	kbuf.buffer = kernel;
> > > > 	kbuf.bufsz = kernel_len;
> > > >
> > > 
> > > And also thanks for posing a fix! The patch LGTM. Can you add a Fixes
> > > tag 'Fixes: bf454ec31add ("kexec_file: allow to place kexec_buf
> > > randomly")' and then send it to kexec@lists.infradead.org? Thanks!
> > 
> > I turned all this into a regular patch and queued it (see below),
> > thanks.  No additional actions are needed.
> > 
> > I'm really not liking that code.  I laboriously verified that all
> > fields of kexec_buf are now initialized, except for `cma'.  Is that a
> > bug?
> > 
> > This function has a call frequency of about 3x per week.  Can we please
> > just memset the whole thing so people don't have to worry about this
> > any more?
> 
> Yeah, adding these trivial patches to mute XXSAN warning is annoying.

The patchset is quite simple, tho:

https://lore.kernel.org/all/20250827-kbuf_all-v1-0-1df9882bb01a@debian.org/


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-27 16:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250502011246.99238-1-coxu@redhat.com>
     [not found] ` <20250502011246.99238-2-coxu@redhat.com>
2025-08-21 11:15   ` [PATCH v9 1/8] kexec_file: allow to place kexec_buf randomly Breno Leitao
2025-08-25  1:18     ` Coiby Xu
2025-08-26  1:05       ` Andrew Morton
2025-08-27 11:38         ` Baoquan He
2025-08-27 13:38           ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).