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

[Bradley Morgan <brads@mainlining.org>]

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!