| Message ID | 20240831094310.4148930-1-lihongbo22@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-next,v2] sh: intc: replace simple_strtoul to kstrtoul | expand |
Hello Hongo, thanks for your patch! On Sat, 2024-08-31 at 17:43 +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. > > Signed-off-by: Hongbo Li <lihongbo22@huawei.com> > > --- > v2: > - Pass the error code returned by kstrtoul() suggested by Geert. > > v1: https://lore.kernel.org/all/98c7b473-0b2b-4e47-83f6-35d9f417bb01@huawei.com/T/ > --- > drivers/sh/intc/userimask.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/sh/intc/userimask.c b/drivers/sh/intc/userimask.c > index abe9091827cd..5d7801b76715 100644 > --- a/drivers/sh/intc/userimask.c > +++ b/drivers/sh/intc/userimask.c > @@ -32,8 +32,10 @@ store_intc_userimask(struct device *dev, > const char *buf, size_t count) > { > unsigned long level; > + int ret = kstrtoul(buf, 10, &level); Could you separate declaration and assignment here to make it more consistent with the rest of the code? > - level = simple_strtoul(buf, NULL, 10); > + if (ret != 0) > + return ret; > > /* > * Minimal acceptable IRQ levels are in the 2 - 16 range, but Thanks, Adrian
On 2024/9/1 2:07, John Paul Adrian Glaubitz wrote: > Hello Hongo, > > thanks for your patch! > > On Sat, 2024-08-31 at 17:43 +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. >> >> Signed-off-by: Hongbo Li <lihongbo22@huawei.com> >> >> --- >> v2: >> - Pass the error code returned by kstrtoul() suggested by Geert. >> >> v1: https://lore.kernel.org/all/98c7b473-0b2b-4e47-83f6-35d9f417bb01@huawei.com/T/ >> --- >> drivers/sh/intc/userimask.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/sh/intc/userimask.c b/drivers/sh/intc/userimask.c >> index abe9091827cd..5d7801b76715 100644 >> --- a/drivers/sh/intc/userimask.c >> +++ b/drivers/sh/intc/userimask.c >> @@ -32,8 +32,10 @@ store_intc_userimask(struct device *dev, >> const char *buf, size_t count) >> { >> unsigned long level; >> + int ret = kstrtoul(buf, 10, &level); > > Could you separate declaration and assignment here to make it more consistent > with the rest of the code? ok, thanks for your advise! Thanks, Hongbo > >> - level = simple_strtoul(buf, NULL, 10); >> + if (ret != 0) >> + return ret; >> >> /* >> * Minimal acceptable IRQ levels are in the 2 - 16 range, but > > Thanks, > Adrian >
diff --git a/drivers/sh/intc/userimask.c b/drivers/sh/intc/userimask.c index abe9091827cd..5d7801b76715 100644 --- a/drivers/sh/intc/userimask.c +++ b/drivers/sh/intc/userimask.c @@ -32,8 +32,10 @@ store_intc_userimask(struct device *dev, const char *buf, size_t count) { unsigned long level; + int ret = kstrtoul(buf, 10, &level); - level = simple_strtoul(buf, NULL, 10); + if (ret != 0) + return ret; /* * Minimal acceptable IRQ levels are in the 2 - 16 range, but
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> --- v2: - Pass the error code returned by kstrtoul() suggested by Geert. v1: https://lore.kernel.org/all/98c7b473-0b2b-4e47-83f6-35d9f417bb01@huawei.com/T/ --- drivers/sh/intc/userimask.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)