[PATCH] kexec: fix stack buffer overflow in update_purgatory()

[PATCH] kexec: fix stack buffer overflow in update_purgatory()

[zhangzikang1992]

From: zhangzikang <zhangzikang@kylinos.cn>

Problem:
A stack buffer overflow exists in "update_purgatory()". The local array "sha256_region region[SHA256_REGIONS]" (size 16) is written during iteration over "info->nr_segments" without boundary checking for the write index "j".

"Add_degment_phys-virt()" only issues a warning when "nr_degments" exceeds "KEXEC_MAX_SGMENTS" (=16) and continues to append segments. Therefore, "update_purgation()" may be called, causing "nr_degments" to be much larger than "SHA256-REGIONS", resulting in "region [j]" being written outside of the 16 item stack array. This type of out of bounds write will damage the Canary stack and adjacent stack frames.

Impact:
Stack buffer overflow causing process crash (resulting in denial of service through Canary termination); The potential for controllable stack destruction depends on the content of the image.

Fix:
Before writing to "region[j]" in "update_purgatory()", add a bounds check on "j" to ensure that writing stops immediately when "j" reaches "SHA256_REGIONS", preventing out-of-bounds writes from corrupting adjacent global variables.

Signed-off-by: zhangzikang <zhangzikang@kylinos.cn>
---
 kexec/kexec.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index c9e4bcb..732058e 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -728,6 +728,11 @@ static void update_purgatory(struct kexec_info *info)
 			sha256_update(&ctx, null_buf, bytes);
 			nullsz -= bytes;
 		}
+		if (j >= SHA256_REGIONS) {
+			fprintf(stderr, "Warning: SHA256 region limit reached. "
+				"Skipping remaining segments from checksum region list\n");
+			break;
+		}
 		region[j].start = (unsigned long) info->segment[i].mem;
 		region[j].len   = info->segment[i].memsz;
 		j++;
-- 
2.43.0

Re: [PATCH] kexec: fix stack buffer overflow in update_purgatory()

[Simon Horman]

On Fri, Oct 30, 2026 at 11:09:18AM +0800, zhangzikang1992@126.com wrote:
> From: zhangzikang <zhangzikang@kylinos.cn>
> 
> Problem:
> A stack buffer overflow exists in "update_purgatory()". The local array "sha256_region region[SHA256_REGIONS]" (size 16) is written during iteration over "info->nr_segments" without boundary checking for the write index "j".
> 
> "Add_degment_phys-virt()" only issues a warning when "nr_degments" exceeds "KEXEC_MAX_SGMENTS" (=16) and continues to append segments. Therefore, "update_purgation()" may be called, causing "nr_degments" to be much larger than "SHA256-REGIONS", resulting in "region [j]" being written outside of the 16 item stack array. This type of out of bounds write will damage the Canary stack and adjacent stack frames.
> 
> Impact:
> Stack buffer overflow causing process crash (resulting in denial of service through Canary termination); The potential for controllable stack destruction depends on the content of the image.
> 
> Fix:
> Before writing to "region[j]" in "update_purgatory()", add a bounds check on "j" to ensure that writing stops immediately when "j" reaches "SHA256_REGIONS", preventing out-of-bounds writes from corrupting adjacent global variables.
> 
> Signed-off-by: zhangzikang <zhangzikang@kylinos.cn>
> ---
>  kexec/kexec.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index c9e4bcb..732058e 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -728,6 +728,11 @@ static void update_purgatory(struct kexec_info *info)
>  			sha256_update(&ctx, null_buf, bytes);
>  			nullsz -= bytes;
>  		}
> +		if (j >= SHA256_REGIONS) {
> +			fprintf(stderr, "Warning: SHA256 region limit reached. "
> +				"Skipping remaining segments from checksum region list\n");
> +			break;
> +		}
>  		region[j].start = (unsigned long) info->segment[i].mem;
>  		region[j].len   = info->segment[i].memsz;
>  		j++;

Hi Zikang,

Thanks for your patch. I agree this is a bug.
And that your patch avoids kexec-tools crashing

However, I think by skipping regions as your patch proposes we move the
burden of failing what is an invalid operation to the load call, which will
fail because the Kernel enforces a 16 segment limit. And, moreover, it's
probably not best to create a mismatch between the calculated digest and
the populated regions, which appears to be possible with your patch.

What I would suggest is this:

1. Add a segment limit check to add_segment_phys_virt,
   logging an error and returning -1 if the check fails.

2. As a failsafe mechanism, add a check to update_purgatory,
   but place it as early in the loop as possible, and
   call die() rather than just logging an error and continuing.

Re: [PATCH] kexec: fix stack buffer overflow in update_purgatory()

[Simon Horman]

On Tue, Nov 10, 2026 at 10:58:41AM +0000, Simon Horman wrote:
> On Fri, Oct 30, 2026 at 11:09:18AM +0800, zhangzikang1992@126.com wrote:
> > From: zhangzikang <zhangzikang@kylinos.cn>
> > 
> > Problem:
> > A stack buffer overflow exists in "update_purgatory()". The local array "sha256_region region[SHA256_REGIONS]" (size 16) is written during iteration over "info->nr_segments" without boundary checking for the write index "j".
> > 
> > "Add_degment_phys-virt()" only issues a warning when "nr_degments" exceeds "KEXEC_MAX_SGMENTS" (=16) and continues to append segments. Therefore, "update_purgation()" may be called, causing "nr_degments" to be much larger than "SHA256-REGIONS", resulting in "region [j]" being written outside of the 16 item stack array. This type of out of bounds write will damage the Canary stack and adjacent stack frames.
> > 
> > Impact:
> > Stack buffer overflow causing process crash (resulting in denial of service through Canary termination); The potential for controllable stack destruction depends on the content of the image.
> > 
> > Fix:
> > Before writing to "region[j]" in "update_purgatory()", add a bounds check on "j" to ensure that writing stops immediately when "j" reaches "SHA256_REGIONS", preventing out-of-bounds writes from corrupting adjacent global variables.
> > 
> > Signed-off-by: zhangzikang <zhangzikang@kylinos.cn>
> > ---
> >  kexec/kexec.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index c9e4bcb..732058e 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -728,6 +728,11 @@ static void update_purgatory(struct kexec_info *info)
> >  			sha256_update(&ctx, null_buf, bytes);
> >  			nullsz -= bytes;
> >  		}
> > +		if (j >= SHA256_REGIONS) {
> > +			fprintf(stderr, "Warning: SHA256 region limit reached. "
> > +				"Skipping remaining segments from checksum region list\n");
> > +			break;
> > +		}
> >  		region[j].start = (unsigned long) info->segment[i].mem;
> >  		region[j].len   = info->segment[i].memsz;
> >  		j++;
> 
> Hi Zikang,
> 
> Thanks for your patch. I agree this is a bug.
> And that your patch avoids kexec-tools crashing
> 
> However, I think by skipping regions as your patch proposes we move the
> burden of failing what is an invalid operation to the load call, which will
> fail because the Kernel enforces a 16 segment limit. And, moreover, it's
> probably not best to create a mismatch between the calculated digest and
> the populated regions, which appears to be possible with your patch.
> 
> What I would suggest is this:
> 
> 1. Add a segment limit check to add_segment_phys_virt,
>    logging an error and returning -1 if the check fails.
> 
> 2. As a failsafe mechanism, add a check to update_purgatory,
>    but place it as early in the loop as possible, and
>    call die() rather than just logging an error and continuing.

Hi again,

I tried to code-up my own suggestion and realised that point 1 was
slightly off the mark: add_segment_phys_virt doesn't have a return value,
and there is already a segment check present.

I think what would be best is to move the check, because it seems
to come too late to avoid a possible overrun within that function,
and die() if the check fails.

Something like this:

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 99ec3ff3c373..b1d2b595682e 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -363,6 +363,10 @@ void add_segment_phys_virt(struct kexec_info *info,
 			(void *)base, (void *)last);
 	}
 
+	if (info->nr_segments >= KEXEC_MAX_SEGMENTS) {
+		die("Kernel segment limit reached.\n");
+	}
+
 	size = (info->nr_segments + 1) * sizeof(info->segment[0]);
 	info->segment = xrealloc(info->segment, size);
 	info->segment[info->nr_segments].buf   = buf;
@@ -370,10 +374,6 @@ void add_segment_phys_virt(struct kexec_info *info,
 	info->segment[info->nr_segments].mem   = (void *)base;
 	info->segment[info->nr_segments].memsz = memsz;
 	info->nr_segments++;
-	if (info->nr_segments > KEXEC_MAX_SEGMENTS) {
-		fprintf(stderr, "Warning: kernel segment limit reached. "
-			"This will likely fail\n");
-	}
 }
 
 unsigned long add_buffer_phys_virt(struct kexec_info *info,
@@ -718,6 +718,10 @@ static void update_purgatory(struct kexec_info *info)
 			continue;
 		}
 
+		if (j >= SHA256_REGIONS) {
+			die("update_purgatory: Exceeded maximum SHA256 regions\n");
+		}
+
 		sha256_update(&ctx, info->segment[i].buf,
 			      info->segment[i].bufsz);
 		nullsz = info->segment[i].memsz - info->segment[i].bufsz;

Re:Re: [PATCH] kexec: fix stack buffer overflow in update_purgatory()

[张子康]

Hi Simon,

Thanks for your review and for taking the time to share your suggestion — I really appreciate the detailed feedback.

I did consider adding a die when the 16-segment limit is reached in add_segment_phys_virt, but after looking at the overall design I think the current approach of printing a warning without terminating may be the reasonable one, for two reasons:

1. KEXEC_MAX_SEGMENTS in kexec-tools is only an expectation on the userspace tool side. Printing a warning and then letting the kernel perform the final validation and return an error keeps the flow complete. If we changed this to die, kexec-tools would exit before even issuing the system call. Although the outcome is failure either way, what the user sees would be the tool giving up on its own, rather than the kernel explicitly reporting "invalid argument" (EINVAL).

2. The value of KEXEC_MAX_SEGMENTS has historically been increased from 8 to 16 in kexec-tools, with KEXEC_SEGMENT_MAX on the kernel side adjusted in tandem. This limit is inherently arbitrary, so in update_purgatory merely avoiding out-of-bounds writes and emitting a warning that the region limit has been reached — and then deferring final validation to the kernel — is more flexible than a hard termination, and can accommodate the actual limits of different kernel versions.

Please let me know what you think.




At 2026-11-10 19:21:51, "Simon Horman" <horms@kernel.org> wrote:
>On Tue, Nov 10, 2026 at 10:58:41AM +0000, Simon Horman wrote:
>> On Fri, Oct 30, 2026 at 11:09:18AM +0800, zhangzikang1992@126.com wrote:
>> > From: zhangzikang <zhangzikang@kylinos.cn>
>> > 
>> > Problem:
>> > A stack buffer overflow exists in "update_purgatory()". The local array "sha256_region region[SHA256_REGIONS]" (size 16) is written during iteration over "info->nr_segments" without boundary checking for the write index "j".
>> > 
>> > "Add_degment_phys-virt()" only issues a warning when "nr_degments" exceeds "KEXEC_MAX_SGMENTS" (=16) and continues to append segments. Therefore, "update_purgation()" may be called, causing "nr_degments" to be much larger than "SHA256-REGIONS", resulting in "region [j]" being written outside of the 16 item stack array. This type of out of bounds write will damage the Canary stack and adjacent stack frames.
>> > 
>> > Impact:
>> > Stack buffer overflow causing process crash (resulting in denial of service through Canary termination); The potential for controllable stack destruction depends on the content of the image.
>> > 
>> > Fix:
>> > Before writing to "region[j]" in "update_purgatory()", add a bounds check on "j" to ensure that writing stops immediately when "j" reaches "SHA256_REGIONS", preventing out-of-bounds writes from corrupting adjacent global variables.
>> > 
>> > Signed-off-by: zhangzikang <zhangzikang@kylinos.cn>
>> > ---
>> >  kexec/kexec.c | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> > 
>> > diff --git a/kexec/kexec.c b/kexec/kexec.c
>> > index c9e4bcb..732058e 100644
>> > --- a/kexec/kexec.c
>> > +++ b/kexec/kexec.c
>> > @@ -728,6 +728,11 @@ static void update_purgatory(struct kexec_info *info)
>> >  			sha256_update(&ctx, null_buf, bytes);
>> >  			nullsz -= bytes;
>> >  		}
>> > +		if (j >= SHA256_REGIONS) {
>> > +			fprintf(stderr, "Warning: SHA256 region limit reached. "
>> > +				"Skipping remaining segments from checksum region list\n");
>> > +			break;
>> > +		}
>> >  		region[j].start = (unsigned long) info->segment[i].mem;
>> >  		region[j].len   = info->segment[i].memsz;
>> >  		j++;
>> 
>> Hi Zikang,
>> 
>> Thanks for your patch. I agree this is a bug.
>> And that your patch avoids kexec-tools crashing
>> 
>> However, I think by skipping regions as your patch proposes we move the
>> burden of failing what is an invalid operation to the load call, which will
>> fail because the Kernel enforces a 16 segment limit. And, moreover, it's
>> probably not best to create a mismatch between the calculated digest and
>> the populated regions, which appears to be possible with your patch.
>> 
>> What I would suggest is this:
>> 
>> 1. Add a segment limit check to add_segment_phys_virt,
>>    logging an error and returning -1 if the check fails.
>> 
>> 2. As a failsafe mechanism, add a check to update_purgatory,
>>    but place it as early in the loop as possible, and
>>    call die() rather than just logging an error and continuing.
>
>Hi again,
>
>I tried to code-up my own suggestion and realised that point 1 was
>slightly off the mark: add_segment_phys_virt doesn't have a return value,
>and there is already a segment check present.
>
>I think what would be best is to move the check, because it seems
>to come too late to avoid a possible overrun within that function,
>and die() if the check fails.
>
>Something like this:
>
>diff --git a/kexec/kexec.c b/kexec/kexec.c
>index 99ec3ff3c373..b1d2b595682e 100644
>--- a/kexec/kexec.c
>+++ b/kexec/kexec.c
>@@ -363,6 +363,10 @@ void add_segment_phys_virt(struct kexec_info *info,
> 			(void *)base, (void *)last);
> 	}
> 
>+	if (info->nr_segments >= KEXEC_MAX_SEGMENTS) {
>+		die("Kernel segment limit reached.\n");
>+	}
>+
> 	size = (info->nr_segments + 1) * sizeof(info->segment[0]);
> 	info->segment = xrealloc(info->segment, size);
> 	info->segment[info->nr_segments].buf   = buf;
>@@ -370,10 +374,6 @@ void add_segment_phys_virt(struct kexec_info *info,
> 	info->segment[info->nr_segments].mem   = (void *)base;
> 	info->segment[info->nr_segments].memsz = memsz;
> 	info->nr_segments++;
>-	if (info->nr_segments > KEXEC_MAX_SEGMENTS) {
>-		fprintf(stderr, "Warning: kernel segment limit reached. "
>-			"This will likely fail\n");
>-	}
> }
> 
> unsigned long add_buffer_phys_virt(struct kexec_info *info,
>@@ -718,6 +718,10 @@ static void update_purgatory(struct kexec_info *info)
> 			continue;
> 		}
> 
>+		if (j >= SHA256_REGIONS) {
>+			die("update_purgatory: Exceeded maximum SHA256 regions\n");
>+		}
>+
> 		sha256_update(&ctx, info->segment[i].buf,
> 			      info->segment[i].bufsz);
> 		nullsz = info->segment[i].memsz - info->segment[i].bufsz;

Re: [PATCH] kexec: fix stack buffer overflow in update_purgatory()

[Simon Horman]

On Tue, Nov 10, 2026 at 08:36:12PM +0800, 张子康  wrote:
> 
> Hi Simon,
> 
> Thanks for your review and for taking the time to share your suggestion — I really appreciate the detailed feedback.
> 
> I did consider adding a die when the 16-segment limit is reached in add_segment_phys_virt, but after looking at the overall design I think the current approach of printing a warning without terminating may be the reasonable one, for two reasons:
> 
> 1. KEXEC_MAX_SEGMENTS in kexec-tools is only an expectation on the userspace tool side. Printing a warning and then letting the kernel perform the final validation and return an error keeps the flow complete. If we changed this to die, kexec-tools would exit before even issuing the system call. Although the outcome is failure either way, what the user sees would be the tool giving up on its own, rather than the kernel explicitly reporting "invalid argument" (EINVAL).
> 
> 2. The value of KEXEC_MAX_SEGMENTS has historically been increased from 8 to 16 in kexec-tools, with KEXEC_SEGMENT_MAX on the kernel side adjusted in tandem. This limit is inherently arbitrary, so in update_purgatory merely avoiding out-of-bounds writes and emitting a warning that the region limit has been reached — and then deferring final validation to the kernel — is more flexible than a hard termination, and can accommodate the actual limits of different kernel versions.
> 
> Please let me know what you think.

Hi Zikang,

Thanks for the response, likewise much appreciated.

I understand what you are saying about the desirability of the Kernel
enforcing the kernel limit. But to my mind we the kexec-tools limit is also
a limit, even if it has the same value as the kernel limit.  And it is
appropriate for kexec-tools to enforce the limit it has.

I think that if kexec-tools used dynamic buffers, and thus had no limit,
then we could happily leave it up to the kernel to enforce what would then
be the only limit. But I also don't think it's worth moving kexec-tools to
use dynamic buffers to achieve this.

So I do think, given the implementation of kexec-tools, that the approach I
suggested is a good way to go.