Re: [PATCH] reboot: validate reboot CPU against the possible mask

[Bradley Morgan <brads@mainlining.org>]

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!