On Fri, Oct 16, 2020 at 2:20 PM Petr Mladek pmladek@suse.com wrote:
On Wed 2020-10-14 23:27:46, Matteo Croce wrote:
From: Matteo Croce mcroce@microsoft.com
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 know, but it's the only function that I know which has the same behaviour of simple_strtoul(). Other than sscanf, which uses simple_strtoul(). I didn't know that simple_strtoul() is no longer obsolete, I will revert to it then.
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.
Ok, thanks!