Re: [PATCH] reboot: fix parsing of reboot cpu number

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux