| Message ID | 20240830080220.3545142-1-lihongbo22@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] watchdog: cpwd: replace simple_strtoul to kstrtoul | expand |
On Fri, Aug 30, 2024 at 04:02:20PM +0800, Hongbo Li wrote: > The function simple_strtoul performs no error checking > in scenarios where the input value overflows the intended > output variable. > > We can replace the use of the simple_strtoul with the safer > alternatives kstrtoul. For fail case, we also print the extra > message. > I really don't see value in this patch, sorry. If the driver is still in use somewhere, it would be much more valuable to convert it to use the watchdog subsystem API, which would also address the above. In general people should refrain from "improving" old watchdog drivers unless they are also converted to use the watchdog subsystem API, or unless a real bug is fixed. Guenter
On 2024/8/31 4:09, Guenter Roeck wrote: > On Fri, Aug 30, 2024 at 04:02:20PM +0800, Hongbo Li wrote: >> The function simple_strtoul performs no error checking >> in scenarios where the input value overflows the intended >> output variable. >> >> We can replace the use of the simple_strtoul with the safer >> alternatives kstrtoul. For fail case, we also print the extra >> message. >> > > I really don't see value in this patch, sorry. If the driver is still > in use somewhere, it would be much more valuable to convert it to use > the watchdog subsystem API, which would also address the above. > > In general people should refrain from "improving" old watchdog drivers > unless they are also converted to use the watchdog subsystem API, > or unless a real bug is fixed. Sorry, I haven't noticed this. Thanks, Hongbo > > Guenter
diff --git a/drivers/watchdog/cpwd.c b/drivers/watchdog/cpwd.c index 901b94d456db..978bc6c87a35 100644 --- a/drivers/watchdog/cpwd.c +++ b/drivers/watchdog/cpwd.c @@ -550,8 +550,14 @@ static int cpwd_probe(struct platform_device *op) p->reboot = (prop_val ? true : false); str_prop = of_get_property(options, "watchdog-timeout", NULL); - if (str_prop) - p->timeout = simple_strtoul(str_prop, NULL, 10); + if (str_prop) { + err = kstrtoul(str_prop, 10, &p->timeout); + if (err != 0) { + pr_err("Unable to parse watchdog-timeout\n"); + of_node_put(options); + goto out_iounmap; + } + } of_node_put(options);
The function simple_strtoul performs no error checking in scenarios where the input value overflows the intended output variable. We can replace the use of the simple_strtoul with the safer alternatives kstrtoul. For fail case, we also print the extra message. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> --- drivers/watchdog/cpwd.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)