mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] reboot: validate reboot CPU against the possible mask
@ 2026-10-31  6:06 Avi Weiss
  2026-10-31 14:13 ` Bradley Morgan
  0 siblings, 1 reply; 3+ messages in thread
From: Avi Weiss @ 2026-10-31  6:06 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Matteo Croce, Petr Mladek, Bradley Morgan, linux-kernel

reboot_setup() and cpu_store() compare the requested CPU ID against
num_possible_cpus(), which is a count rather than the highest possible
CPU ID. On sparse masks, this rejects valid high-numbered CPUs and
accepts impossible CPU IDs.

Validate the CPU ID against nr_cpu_ids and cpu_possible() instead.
Reject negative parsed values before the mask lookup and update the
error message to avoid assuming contiguous CPU numbering.

On an arm64 QEMU guest with possible CPUs 0,2-3, reboot=s3 selected
CPU 0 and sysfs rejected CPU 3 with -ERANGE. With the fix, reboot=s3
selects CPU 3 and sysfs accepts it, while both interfaces reject
impossible CPU 1.

Fixes: df5b0ab3e08a ("reboot: fix overflow parsing reboot cpu number")
Fixes: 2c622ed0eaa3 ("reboot: allow to specify reboot mode via sysfs")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
---
 kernel/reboot.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/reboot.c b/kernel/reboot.c
index 9a84401adde5..7b09116376e3 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -1146,10 +1146,9 @@ static int __init reboot_setup(char *str)
 			if (isdigit(str[0])) {
 				int cpu = simple_strtoul(str, NULL, 0);
 
-				if (cpu >= num_possible_cpus()) {
-					pr_err("Ignoring the CPU number in reboot= option. "
-					"CPU %d exceeds possible cpu number %d\n",
-					cpu, num_possible_cpus());
+				if (cpu < 0 || cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
+					pr_err("Ignoring invalid CPU %d in reboot= option\n",
+					       cpu);
 					break;
 				}
 				reboot_cpu = cpu;
@@ -1351,7 +1350,7 @@ static ssize_t cpu_store(struct kobject *kobj, struct kobj_attribute *attr,
 	if (rc)
 		return rc;
 
-	if (cpunum >= num_possible_cpus())
+	if (cpunum >= nr_cpu_ids || !cpu_possible(cpunum))
 		return -ERANGE;
 
 	reboot_default = 0;

base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
-- 
2.43.0


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

* Re: [PATCH] reboot: validate reboot CPU against the possible mask
  2026-10-31  6:06 [PATCH] reboot: validate reboot CPU against the possible mask Avi Weiss
@ 2026-10-31 14:13 ` Bradley Morgan
       [not found]   ` <CA+jvY9F9DS4SwAg=xPS-8GRee_01V5LpbCpSWdP581MOuwBz_A@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Bradley Morgan @ 2026-10-31 14:13 UTC (permalink / raw)
  To: Avi Weiss, Andrew Morton; +Cc: Matteo Croce, Petr Mladek, linux-kernel

On 31 October 2026 06:06:03 GMT, Avi Weiss <thnkslprpt@gmail.com> wrote:
>reboot_setup() and cpu_store() compare the requested CPU ID against
>num_possible_cpus(), which is a count rather than the highest possible
>CPU ID. On sparse masks, this rejects valid high-numbered CPUs and
>accepts impossible CPU IDs.
>
>Validate the CPU ID against nr_cpu_ids and cpu_possible() instead.
>Reject negative parsed values before the mask lookup and update the
>error message to avoid assuming contiguous CPU numbering.
>
>On an arm64 QEMU guest with possible CPUs 0,2-3, reboot=s3 selected
>CPU 0 and sysfs rejected CPU 3 with -ERANGE. With the fix, reboot=s3
>selects CPU 3 and sysfs accepts it, while both interfaces reject
>impossible CPU 1.

Hmm? Did you have to hack the dtb?

>
>Fixes: df5b0ab3e08a ("reboot: fix overflow parsing reboot cpu number")
>Fixes: 2c622ed0eaa3 ("reboot: allow to specify reboot mode via sysfs")
>Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
>---
> kernel/reboot.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/kernel/reboot.c b/kernel/reboot.c
>index 9a84401adde5..7b09116376e3 100644
>--- a/kernel/reboot.c
>+++ b/kernel/reboot.c
>@@ -1146,10 +1146,9 @@ static int __init reboot_setup(char *str)
> 			if (isdigit(str[0])) {
> 				int cpu = simple_strtoul(str, NULL, 0);
> 
>-				if (cpu >= num_possible_cpus()) {
>-					pr_err("Ignoring the CPU number in reboot= option. "
>-					"CPU %d exceeds possible cpu number %d\n",
>-					cpu, num_possible_cpus());
>+				if (cpu < 0 || cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
>+					pr_err("Ignoring invalid CPU %d in reboot= option\n",
>+					       cpu);

Could the parse be unsigned? simple_strtoul() returns an unsigned long,
the int cast cuts it down, so reboot=s4294967296 wraps to 0 on 64 bit
and you reboot on CPU 0 anyway. And cpu < 0 goes away too, here's a
something you could apply on top of Ur patch.

diff --git a/kernel/reboot.c b/kernel/reboot.c
index 7b09116376e35..4f28f7eb21a13 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -1144,10 +1144,10 @@ static int __init reboot_setup(char *str)
 			str += str[1] == 'm' && str[2] == 'p' ? 3 : 1;
 
 			if (isdigit(str[0])) {
-				int cpu = simple_strtoul(str, NULL, 0);
+				unsigned long cpu = simple_strtoul(str, NULL, 0);
 
-				if (cpu < 0 || cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
-					pr_err("Ignoring invalid CPU %d in reboot= option\n",
+				if (cpu >= nr_cpu_ids || !cpu_possible(cpu)) {
+					pr_err("Ignoring invalid CPU %lu in reboot= option\n",
 					       cpu);
 					break;
 				}

kstrtoul() chokes on the ,... tail, so simple_strtoul() has to stay.

> 					break;
> 				}
> 				reboot_cpu = cpu;
>@@ -1351,7 +1350,7 @@ static ssize_t cpu_store(struct kobject *kobj, struct kobj_attribute *attr,
> 	if (rc)
> 		return rc;
> 
>-	if (cpunum >= num_possible_cpus())
>+	if (cpunum >= nr_cpu_ids || !cpu_possible(cpunum))
> 		return -ERANGE;
> 
> 	reboot_default = 0;

One more thing, and this is the suspicious part. Youve sent 101
patches since july, 15 today alone, across 59 different subsystems and
drivers, and
lore has the whole run of them. 

Id reckon you'd be using a LLM to develop them, and that's fine, but other
subsystems may not like this kind of thing, so perhaps be more considerate
of others when submitting the patches?

Perhaps Google maintainer burnout and that'll help you.

>
>base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
>

--- Thanks!

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

* Re: [PATCH] reboot: validate reboot CPU against the possible mask
       [not found]   ` <CA+jvY9F9DS4SwAg=xPS-8GRee_01V5LpbCpSWdP581MOuwBz_A@mail.gmail.com>
@ 2026-10-31 15:25     ` Bradley Morgan
  0 siblings, 0 replies; 3+ messages in thread
From: Bradley Morgan @ 2026-10-31 15:25 UTC (permalink / raw)
  To: Avi; +Cc: Andrew Morton, Matteo Croce, Petr Mladek, linux-kernel

On 31 October 2026 14:58:04 GMT, Avi <thnkslprpt@gmail.com> wrote:
>Hi Bradley,
>Sure no worries - I'll check your suggestions and after I re-validate it
>I'll submit a v2.
>Regarding the patches - yes I'm working through a big list of (potential)
>bugs I had flagged from last year, but have a chance now on some holidays
>to run through them.
>I only submit things I'm confident of and have reproduced.
>I'll take your advice on board.
>Cheers,
>Avi

Please don't lie. It's not embarrassing to use AI for things...
Really it's not, anyone who embarrasses or says your wrong for using AI is
wrong in my opinion.

>
>On Sat, Oct 31, 2026 at 4:13 PM Bradley Morgan <brads@mainlining.org>
>wrote:
>
>> On 31 October 2026 06:06:03 GMT, Avi Weiss <thnkslprpt@gmail.com> wrote:
>> >reboot_setup() and cpu_store() compare the requested CPU ID against
>> >num_possible_cpus(), which is a count rather than the highest possible
>> >CPU ID. On sparse masks, this rejects valid high-numbered CPUs and
>> >accepts impossible CPU IDs.
>> >
>> >Validate the CPU ID against nr_cpu_ids and cpu_possible() instead.
>> >Reject negative parsed values before the mask lookup and update the
>> >error message to avoid assuming contiguous CPU numbering.
>> >
>> >On an arm64 QEMU guest with possible CPUs 0,2-3, reboot=s3 selected
>> >CPU 0 and sysfs rejected CPU 3 with -ERANGE. With the fix, reboot=s3
>> >selects CPU 3 and sysfs accepts it, while both interfaces reject
>> >impossible CPU 1.
>>
>> Hmm? Did you have to hack the dtb?
>>
>> >
>> >Fixes: df5b0ab3e08a ("reboot: fix overflow parsing reboot cpu number")
>> >Fixes: 2c622ed0eaa3 ("reboot: allow to specify reboot mode via sysfs")
>> >Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
>> >---
>> > kernel/reboot.c | 9 ++++-----
>> > 1 file changed, 4 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/kernel/reboot.c b/kernel/reboot.c
>> >index 9a84401adde5..7b09116376e3 100644
>> >--- a/kernel/reboot.c
>> >+++ b/kernel/reboot.c
>> >@@ -1146,10 +1146,9 @@ static int __init reboot_setup(char *str)
>> >                       if (isdigit(str[0])) {
>> >                               int cpu = simple_strtoul(str, NULL, 0);
>> >
>> >-                              if (cpu >= num_possible_cpus()) {
>> >-                                      pr_err("Ignoring the CPU number
>in
>> reboot= option. "
>> >-                                      "CPU %d exceeds possible cpu
>> number %d\n",
>> >-                                      cpu, num_possible_cpus());
>> >+                              if (cpu < 0 || cpu >= nr_cpu_ids ||
>> !cpu_possible(cpu)) {
>> >+                                      pr_err("Ignoring invalid CPU %d
>in
>> reboot= option\n",
>> >+                                             cpu);
>>
>> Could the parse be unsigned? simple_strtoul() returns an unsigned long,
>> the int cast cuts it down, so reboot=s4294967296 wraps to 0 on 64 bit
>> and you reboot on CPU 0 anyway. And cpu < 0 goes away too, here's a
>> something you could apply on top of Ur patch.
>>
>> diff --git a/kernel/reboot.c b/kernel/reboot.c
>> index 7b09116376e35..4f28f7eb21a13 100644
>> --- a/kernel/reboot.c
>> +++ b/kernel/reboot.c
>> @@ -1144,10 +1144,10 @@ static int __init reboot_setup(char *str)
>>                         str += str[1] == 'm' && str[2] == 'p' ? 3 : 1;
>>
>>                         if (isdigit(str[0])) {
>> -                               int cpu = simple_strtoul(str, NULL, 0);
>> +                               unsigned long cpu = simple_strtoul(str,
>> NULL, 0);
>>
>> -                               if (cpu < 0 || cpu >= nr_cpu_ids ||
>> !cpu_possible(cpu)) {
>> -                                       pr_err("Ignoring invalid CPU %d
>in
>> reboot= option\n",
>> +                               if (cpu >= nr_cpu_ids ||
>> !cpu_possible(cpu)) {
>> +                                       pr_err("Ignoring invalid CPU %lu
>> in reboot= option\n",
>>                                                cpu);
>>                                         break;
>>                                 }
>>
>> kstrtoul() chokes on the ,... tail, so simple_strtoul() has to stay.
>>
>> >                                       break;
>> >                               }
>> >                               reboot_cpu = cpu;
>> >@@ -1351,7 +1350,7 @@ static ssize_t cpu_store(struct kobject *kobj,
>> struct kobj_attribute *attr,
>> >       if (rc)
>> >               return rc;
>> >
>> >-      if (cpunum >= num_possible_cpus())
>> >+      if (cpunum >= nr_cpu_ids || !cpu_possible(cpunum))
>> >               return -ERANGE;
>> >
>> >       reboot_default = 0;
>>
>> One more thing, and this is the suspicious part. Youve sent 101
>> patches since july, 15 today alone, across 59 different subsystems and
>> drivers, and
>> lore has the whole run of them.
>>
>> Id reckon you'd be using a LLM to develop them, and that's fine, but
>other
>> subsystems may not like this kind of thing, so perhaps be more
>considerate
>> of others when submitting the patches?
>>
>> Perhaps Google maintainer burnout and that'll help you.
>>
>> >
>> >base-commit: 36aad21d50d1af3f6954bb0c988257a908202477
>> >
>>
>> --- Thanks!
>>
>

--- Thanks!

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

end of thread, other threads:[~2026-10-31 15:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-31  6:06 [PATCH] reboot: validate reboot CPU against the possible mask Avi Weiss
2026-10-31 14:13 ` Bradley Morgan
     [not found]   ` <CA+jvY9F9DS4SwAg=xPS-8GRee_01V5LpbCpSWdP581MOuwBz_A@mail.gmail.com>
2026-10-31 15:25     ` Bradley Morgan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®