Re: [PATCH] reboot: fix parsing of reboot cpu number
From: Petr Mladek
Date: Fri Oct 16 2020 - 08:20:44 EST
On Wed 2020-10-14 23:27:46, Matteo Croce wrote:
> From: Matteo Croce <mcroce@xxxxxxxxxxxxx>
>
> The kernel cmdline reboot= argument allows to specify the CPU used
> for rebooting, with the syntax `s####` among the other flags, e.g.
>
> reboot=soft,s4
> reboot=warm,s31,force
>
> In the early days the parsing was done with simple_strtoul(), later
> deprecated in favor of the safer kstrtoint() which handles overflow.
>
> But kstrtoint() returns -EINVAL if there are non-digit characters
> in a string, so if this flag is not the last given, it's silently
> ignored as well as the subsequent ones.
>
> To fix it, use _parse_integer() which still handles overflow, but
> restores the old behaviour of parsing until a non-digit character
> is found.
Hmm, _parse_integer() is an internal function. And even the comment
says "Don't you dare use this function."
I guess the it is because the base must be hardcoded. And
KSTRTOX_OVERFLOW bit must be handled.
I suggest to go back to simple_strtoul(). It is not longer obsolete.
It still exists because it is needed for exactly this purpose,
see the comment in include/linux/kernel.h
The potentional overflow is not a big deal. The result will be
that the system will reboot on another rCPU than expected. But
it might happen also with any typo.
> While at it, limit the CPU number to num_possible_cpus(),
> because setting it to a value lower than INT_MAX but higher
> than NR_CPUS produces the following error on reboot and shutdown:
Great catch! Please, fix this in a separate patch.
Best Regards,
Petr