All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kexec: Fix uninitialized struct kimage *image pointer
@ 2025-12-12  7:16 Qiang Ma
  2025-12-12 10:53 ` Baoquan He
  0 siblings, 1 reply; 8+ messages in thread
From: Qiang Ma @ 2025-12-12  7:16 UTC (permalink / raw)
  To: akpm, bhe; +Cc: kexec, linux-kernel, Qiang Ma

The image is initialized to NULL. Then, after calling kimage_alloc_init,
we can directly goto 'out' because at this time, the kimage_free will
determine whether image is a NULL pointer.

Signed-off-by: Qiang Ma <maqianga@uniontech.com>
---
 kernel/kexec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 28008e3d462e..9bb1f2b6b268 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 	unsigned long i;
 	int ret;
 
+	image = NULL;
+
 	/*
 	 * Because we write directly to the reserved memory region when loading
 	 * crash kernels we need a serialization here to prevent multiple crash
@@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
 
 	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
 	if (ret)
-		goto out_unlock;
+		goto out;
 
 	if (flags & KEXEC_PRESERVE_CONTEXT)
 		image->preserve_context = 1;
-- 
2.20.1



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-12  7:16 [PATCH] kexec: Fix uninitialized struct kimage *image pointer Qiang Ma
@ 2025-12-12 10:53 ` Baoquan He
  2025-12-14 10:23   ` Baoquan He
  2025-12-14 11:35   ` Qiang Ma
  0 siblings, 2 replies; 8+ messages in thread
From: Baoquan He @ 2025-12-12 10:53 UTC (permalink / raw)
  To: Qiang Ma; +Cc: akpm, kexec, linux-kernel

On 12/12/25 at 03:16pm, Qiang Ma wrote:
> The image is initialized to NULL. Then, after calling kimage_alloc_init,
> we can directly goto 'out' because at this time, the kimage_free will
> determine whether image is a NULL pointer.

Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
allocated memory are all freed via out_free_control_pages and
out_free_image accordingly, any place missed? If no, I think the current
code is correctly handled.

> 
> Signed-off-by: Qiang Ma <maqianga@uniontech.com>
> ---
>  kernel/kexec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Baoquan He <bhe@redhat.com>

> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 28008e3d462e..9bb1f2b6b268 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  	unsigned long i;
>  	int ret;
>  
> +	image = NULL;
> +
>  	/*
>  	 * Because we write directly to the reserved memory region when loading
>  	 * crash kernels we need a serialization here to prevent multiple crash
> @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>  
>  	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
>  	if (ret)
> -		goto out_unlock;
> +		goto out;
>  
>  	if (flags & KEXEC_PRESERVE_CONTEXT)
>  		image->preserve_context = 1;
> -- 
> 2.20.1
> 



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-12 10:53 ` Baoquan He
@ 2025-12-14 10:23   ` Baoquan He
  2025-12-14 11:35   ` Qiang Ma
  1 sibling, 0 replies; 8+ messages in thread
From: Baoquan He @ 2025-12-14 10:23 UTC (permalink / raw)
  To: Qiang Ma; +Cc: akpm, kexec, linux-kernel

On 12/12/25 at 06:53pm, Baoquan He wrote:
> On 12/12/25 at 03:16pm, Qiang Ma wrote:
> > The image is initialized to NULL. Then, after calling kimage_alloc_init,
> > we can directly goto 'out' because at this time, the kimage_free will
> > determine whether image is a NULL pointer.
> 
> Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
> allocated memory are all freed via out_free_control_pages and
> out_free_image accordingly, any place missed? If no, I think the current
> code is correctly handled.
> 
> > 
> > Signed-off-by: Qiang Ma <maqianga@uniontech.com>
> > ---
> >  kernel/kexec.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> Acked-by: Baoquan He <bhe@redhat.com>

Cancle this ACK till above question is answered.

> 
> > 
> > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > index 28008e3d462e..9bb1f2b6b268 100644
> > --- a/kernel/kexec.c
> > +++ b/kernel/kexec.c
> > @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> >  	unsigned long i;
> >  	int ret;
> >  
> > +	image = NULL;
> > +
> >  	/*
> >  	 * Because we write directly to the reserved memory region when loading
> >  	 * crash kernels we need a serialization here to prevent multiple crash
> > @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> >  
> >  	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
> >  	if (ret)
> > -		goto out_unlock;
> > +		goto out;
> >  
> >  	if (flags & KEXEC_PRESERVE_CONTEXT)
> >  		image->preserve_context = 1;
> > -- 
> > 2.20.1
> > 
> 



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-12 10:53 ` Baoquan He
  2025-12-14 10:23   ` Baoquan He
@ 2025-12-14 11:35   ` Qiang Ma
  2025-12-15  1:18     ` Baoquan He
  1 sibling, 1 reply; 8+ messages in thread
From: Qiang Ma @ 2025-12-14 11:35 UTC (permalink / raw)
  To: Baoquan He; +Cc: akpm, kexec, linux-kernel


在 2025/12/12 18:53, Baoquan He 写道:
> On 12/12/25 at 03:16pm, Qiang Ma wrote:
>> The image is initialized to NULL. Then, after calling kimage_alloc_init,
>> we can directly goto 'out' because at this time, the kimage_free will
>> determine whether image is a NULL pointer.
> Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
> allocated memory are all freed via out_free_control_pages and
> out_free_image accordingly, any place missed? If no, I think the current
> code is correctly handled.
I rechecked the code and found no omissions.
>> Signed-off-by: Qiang Ma <maqianga@uniontech.com>
>> ---
>>   kernel/kexec.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
> Acked-by: Baoquan He <bhe@redhat.com>
>
>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>> index 28008e3d462e..9bb1f2b6b268 100644
>> --- a/kernel/kexec.c
>> +++ b/kernel/kexec.c
>> @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>   	unsigned long i;
>>   	int ret;
>>   
>> +	image = NULL;
>> +
>>   	/*
>>   	 * Because we write directly to the reserved memory region when loading
>>   	 * crash kernels we need a serialization here to prevent multiple crash
>> @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>   
>>   	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
>>   	if (ret)
>> -		goto out_unlock;
>> +		goto out;
>>   
>>   	if (flags & KEXEC_PRESERVE_CONTEXT)
>>   		image->preserve_context = 1;
>> -- 
>> 2.20.1
>>
>


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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-14 11:35   ` Qiang Ma
@ 2025-12-15  1:18     ` Baoquan He
  2025-12-15  4:41       ` Qiang Ma
  0 siblings, 1 reply; 8+ messages in thread
From: Baoquan He @ 2025-12-15  1:18 UTC (permalink / raw)
  To: Qiang Ma; +Cc: akpm, kexec, linux-kernel

On 12/14/25 at 07:35pm, Qiang Ma wrote:
> 
> 在 2025/12/12 18:53, Baoquan He 写道:
> > On 12/12/25 at 03:16pm, Qiang Ma wrote:
> > > The image is initialized to NULL. Then, after calling kimage_alloc_init,
> > > we can directly goto 'out' because at this time, the kimage_free will
> > > determine whether image is a NULL pointer.
> > Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
> > allocated memory are all freed via out_free_control_pages and
> > out_free_image accordingly, any place missed? If no, I think the current
> > code is correctly handled.
> I rechecked the code and found no omissions.

Hmm, my bad, I didn't say my question clearly. I checked code, didn't
find anything wrong in the current code. In kimage_alloc_init(), the
allocated memory are all freed on failure, no memory leaked. Means you
are fixing correct code.

> > > Signed-off-by: Qiang Ma <maqianga@uniontech.com>
> > > ---
> > >   kernel/kexec.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > Acked-by: Baoquan He <bhe@redhat.com>
> > 
> > > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > > index 28008e3d462e..9bb1f2b6b268 100644
> > > --- a/kernel/kexec.c
> > > +++ b/kernel/kexec.c
> > > @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > >   	unsigned long i;
> > >   	int ret;
> > > +	image = NULL;
> > > +
> > >   	/*
> > >   	 * Because we write directly to the reserved memory region when loading
> > >   	 * crash kernels we need a serialization here to prevent multiple crash
> > > @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > >   	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
> > >   	if (ret)
> > > -		goto out_unlock;
> > > +		goto out;
> > >   	if (flags & KEXEC_PRESERVE_CONTEXT)
> > >   		image->preserve_context = 1;
> > > -- 
> > > 2.20.1
> > > 
> > 
> 



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-15  1:18     ` Baoquan He
@ 2025-12-15  4:41       ` Qiang Ma
  2025-12-15  8:50         ` Baoquan He
  0 siblings, 1 reply; 8+ messages in thread
From: Qiang Ma @ 2025-12-15  4:41 UTC (permalink / raw)
  To: Baoquan He; +Cc: akpm, kexec, linux-kernel


在 2025/12/15 09:18, Baoquan He 写道:
> On 12/14/25 at 07:35pm, Qiang Ma wrote:
>> 在 2025/12/12 18:53, Baoquan He 写道:
>>> On 12/12/25 at 03:16pm, Qiang Ma wrote:
>>>> The image is initialized to NULL. Then, after calling kimage_alloc_init,
>>>> we can directly goto 'out' because at this time, the kimage_free will
>>>> determine whether image is a NULL pointer.
>>> Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
>>> allocated memory are all freed via out_free_control_pages and
>>> out_free_image accordingly, any place missed? If no, I think the current
>>> code is correctly handled.
>> I rechecked the code and found no omissions.
> Hmm, my bad, I didn't say my question clearly. I checked code, didn't
> find anything wrong in the current code. In kimage_alloc_init(), the
> allocated memory are all freed on failure, no memory leaked. Means you
> are fixing correct code.
Oh, I see. I recalled that this fix was in preparation for patch
"kexec: add kexec flag to control debug printing" for kexec_dbg_print
to be reset to false in kimage_free.

In that case, I don't think this patch should be posted separately.
>
>>>> Signed-off-by: Qiang Ma <maqianga@uniontech.com>
>>>> ---
>>>>    kernel/kexec.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>> Acked-by: Baoquan He <bhe@redhat.com>
>>>
>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>>> index 28008e3d462e..9bb1f2b6b268 100644
>>>> --- a/kernel/kexec.c
>>>> +++ b/kernel/kexec.c
>>>> @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>>    	unsigned long i;
>>>>    	int ret;
>>>> +	image = NULL;
>>>> +
>>>>    	/*
>>>>    	 * Because we write directly to the reserved memory region when loading
>>>>    	 * crash kernels we need a serialization here to prevent multiple crash
>>>> @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>>    	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
>>>>    	if (ret)
>>>> -		goto out_unlock;
>>>> +		goto out;
>>>>    	if (flags & KEXEC_PRESERVE_CONTEXT)
>>>>    		image->preserve_context = 1;
>>>> -- 
>>>> 2.20.1
>>>>
>



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-15  4:41       ` Qiang Ma
@ 2025-12-15  8:50         ` Baoquan He
  2025-12-15  9:18           ` Qiang Ma
  0 siblings, 1 reply; 8+ messages in thread
From: Baoquan He @ 2025-12-15  8:50 UTC (permalink / raw)
  To: Qiang Ma; +Cc: akpm, kexec, linux-kernel

On 12/15/25 at 12:41pm, Qiang Ma wrote:
> 
> 在 2025/12/15 09:18, Baoquan He 写道:
> > On 12/14/25 at 07:35pm, Qiang Ma wrote:
> > > 在 2025/12/12 18:53, Baoquan He 写道:
> > > > On 12/12/25 at 03:16pm, Qiang Ma wrote:
> > > > > The image is initialized to NULL. Then, after calling kimage_alloc_init,
> > > > > we can directly goto 'out' because at this time, the kimage_free will
> > > > > determine whether image is a NULL pointer.
> > > > Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
> > > > allocated memory are all freed via out_free_control_pages and
> > > > out_free_image accordingly, any place missed? If no, I think the current
> > > > code is correctly handled.
> > > I rechecked the code and found no omissions.
> > Hmm, my bad, I didn't say my question clearly. I checked code, didn't
> > find anything wrong in the current code. In kimage_alloc_init(), the
> > allocated memory are all freed on failure, no memory leaked. Means you
> > are fixing correct code.
> Oh, I see. I recalled that this fix was in preparation for patch
> "kexec: add kexec flag to control debug printing" for kexec_dbg_print
> to be reset to false in kimage_free.
> 
> In that case, I don't think this patch should be posted separately.

If it's prepared for later patch, it should not be saying it's fixing
issue. People may be confused and try to add it to stable kernel.

> > 
> > > > > Signed-off-by: Qiang Ma <maqianga@uniontech.com>
> > > > > ---
> > > > >    kernel/kexec.c | 4 +++-
> > > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > > Acked-by: Baoquan He <bhe@redhat.com>
> > > > 
> > > > > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > > > > index 28008e3d462e..9bb1f2b6b268 100644
> > > > > --- a/kernel/kexec.c
> > > > > +++ b/kernel/kexec.c
> > > > > @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > > > >    	unsigned long i;
> > > > >    	int ret;
> > > > > +	image = NULL;
> > > > > +
> > > > >    	/*
> > > > >    	 * Because we write directly to the reserved memory region when loading
> > > > >    	 * crash kernels we need a serialization here to prevent multiple crash
> > > > > @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
> > > > >    	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
> > > > >    	if (ret)
> > > > > -		goto out_unlock;
> > > > > +		goto out;
> > > > >    	if (flags & KEXEC_PRESERVE_CONTEXT)
> > > > >    		image->preserve_context = 1;
> > > > > -- 
> > > > > 2.20.1
> > > > > 
> > 
> 



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

* Re: [PATCH] kexec: Fix uninitialized struct kimage *image pointer
  2025-12-15  8:50         ` Baoquan He
@ 2025-12-15  9:18           ` Qiang Ma
  0 siblings, 0 replies; 8+ messages in thread
From: Qiang Ma @ 2025-12-15  9:18 UTC (permalink / raw)
  To: Baoquan He; +Cc: akpm, kexec, linux-kernel


在 2025/12/15 16:50, Baoquan He 写道:
> On 12/15/25 at 12:41pm, Qiang Ma wrote:
>> 在 2025/12/15 09:18, Baoquan He 写道:
>>> On 12/14/25 at 07:35pm, Qiang Ma wrote:
>>>> 在 2025/12/12 18:53, Baoquan He 写道:
>>>>> On 12/12/25 at 03:16pm, Qiang Ma wrote:
>>>>>> The image is initialized to NULL. Then, after calling kimage_alloc_init,
>>>>>> we can directly goto 'out' because at this time, the kimage_free will
>>>>>> determine whether image is a NULL pointer.
>>>>> Rechecked the code flow, in kimage_alloc_init(), if anything wrong, the
>>>>> allocated memory are all freed via out_free_control_pages and
>>>>> out_free_image accordingly, any place missed? If no, I think the current
>>>>> code is correctly handled.
>>>> I rechecked the code and found no omissions.
>>> Hmm, my bad, I didn't say my question clearly. I checked code, didn't
>>> find anything wrong in the current code. In kimage_alloc_init(), the
>>> allocated memory are all freed on failure, no memory leaked. Means you
>>> are fixing correct code.
>> Oh, I see. I recalled that this fix was in preparation for patch
>> "kexec: add kexec flag to control debug printing" for kexec_dbg_print
>> to be reset to false in kimage_free.
>>
>> In that case, I don't think this patch should be posted separately.
> If it's prepared for later patch, it should not be saying it's fixing
> issue. People may be confused and try to add it to stable kernel.
That makes sense. I renamed it: "kexec: Replace the goto out_unlock with 
out" in v2.
>>>>>> Signed-off-by: Qiang Ma <maqianga@uniontech.com>
>>>>>> ---
>>>>>>     kernel/kexec.c | 4 +++-
>>>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>> Acked-by: Baoquan He <bhe@redhat.com>
>>>>>
>>>>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
>>>>>> index 28008e3d462e..9bb1f2b6b268 100644
>>>>>> --- a/kernel/kexec.c
>>>>>> +++ b/kernel/kexec.c
>>>>>> @@ -95,6 +95,8 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>>>>     	unsigned long i;
>>>>>>     	int ret;
>>>>>> +	image = NULL;
>>>>>> +
>>>>>>     	/*
>>>>>>     	 * Because we write directly to the reserved memory region when loading
>>>>>>     	 * crash kernels we need a serialization here to prevent multiple crash
>>>>>> @@ -129,7 +131,7 @@ static int do_kexec_load(unsigned long entry, unsigned long nr_segments,
>>>>>>     	ret = kimage_alloc_init(&image, entry, nr_segments, segments, flags);
>>>>>>     	if (ret)
>>>>>> -		goto out_unlock;
>>>>>> +		goto out;
>>>>>>     	if (flags & KEXEC_PRESERVE_CONTEXT)
>>>>>>     		image->preserve_context = 1;
>>>>>> -- 
>>>>>> 2.20.1
>>>>>>
>



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

end of thread, other threads:[~2025-12-15  9:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-12  7:16 [PATCH] kexec: Fix uninitialized struct kimage *image pointer Qiang Ma
2025-12-12 10:53 ` Baoquan He
2025-12-14 10:23   ` Baoquan He
2025-12-14 11:35   ` Qiang Ma
2025-12-15  1:18     ` Baoquan He
2025-12-15  4:41       ` Qiang Ma
2025-12-15  8:50         ` Baoquan He
2025-12-15  9:18           ` Qiang Ma

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.