linux-kernel - Re: [PATCH V2] misc: replace strict

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 3 Jun 2013 14:01:39 +0300
From:	Andy Shevchenko <andy.shevchenko@...il.com>
To:	Jingoo Han <jg1.han@...sung.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Michael Hennerich <michael.hennerich@...log.com>,
	Eric Piel <eric.piel@...mplin-utc.net>,
	Dimitri Sivanich <sivanich@....com>, Robin Holt <holt@....com>
Subject: Re: [PATCH V2] misc: replace strict_strtoul() with kstrtoul()

On Mon, Jun 3, 2013 at 12:08 PM, Jingoo Han <jg1.han@...sung.com> wrote:
> The usage of strict_strtoul() is not preferred, because
> strict_strtoul() is obsolete. Thus, kstrtoul() should be
> used.
>
> Signed-off-by: Jingoo Han <jg1.han@...sung.com>

There are few issues. Afrter addressing them take my
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

> --- a/drivers/misc/isl29003.c
> +++ b/drivers/misc/isl29003.c
> @@ -208,8 +208,9 @@ static ssize_t isl29003_store_range(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 3))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 3);
> +       if (ret)
> +               return ret;

It's not correct.
You have to get return code from kstrtoul() separately and use it,
after check top boundary and return -EINVAL.

Three more cases below.

> @@ -239,8 +240,9 @@ static ssize_t isl29003_store_resolution(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 3))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 3);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_resolution(client, val);
>         if (ret < 0)
> @@ -267,8 +269,9 @@ static ssize_t isl29003_store_mode(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 2))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 2);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_mode(client, val);
>         if (ret < 0)
> @@ -298,8 +301,9 @@ static ssize_t isl29003_store_power_state(struct device *dev,
>         unsigned long val;
>         int ret;
>
> -       if ((strict_strtoul(buf, 10, &val) < 0) || (val > 1))
> -               return -EINVAL;
> +       ret = (kstrtoul(buf, 10, &val) < 0) || (val > 1);
> +       if (ret)
> +               return ret;
>
>         ret = isl29003_set_power_state(client, val);
>         return ret ? ret : count;

> --- a/drivers/misc/sgi-gru/gruprocfs.c
> +++ b/drivers/misc/sgi-gru/gruprocfs.c
> @@ -161,14 +161,17 @@ static ssize_t options_write(struct file *file, const char __user *userbuf,
>                              size_t count, loff_t *data)
>  {
>         char buf[20];
> +       int ret;
>
>         if (count >= sizeof(buf))
>                 return -EINVAL;
>         if (copy_from_user(buf, userbuf, count))
>                 return -EFAULT;
>         buf[count] = '\0';
> -       if (strict_strtoul(buf, 0, &gru_options))
> -               return -EINVAL;
> +
> +       ret = kstrtoul(buf, 0, &gru_options);

kstrtoul_from_user(), please.

> +       if (ret)
> +               return ret;
>
>         return count;
>  }

--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/