2021-05-26 09:13:05

by Chen Huang

Subject: [PATCH -next 1/3] powerpc/rtas: Replaced simple_strtoull() with kstrtoull()

The simple_strtoull() function is deprecated in some situation, since
it does not check for the range overflow, use kstrtoull() instead.

Signed-off-by: Chen Huang <[email protected]>
---
arch/powerpc/kernel/rtas-proc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/rtas-proc.c b/arch/powerpc/kernel/rtas-proc.c
index 6857a5b0a1c3..117886782ebd 100644
--- a/arch/powerpc/kernel/rtas-proc.c
+++ b/arch/powerpc/kernel/rtas-proc.c
@@ -259,7 +259,6 @@ __initcall(proc_rtas_init);
static int parse_number(const char __user *p, size_t count, u64 *val)
{
char buf[40];
- char *end;

if (count > 39)
return -EINVAL;
@@ -269,11 +268,7 @@ static int parse_number(const char __user *p, size_t count, u64 *val)

buf[count] = 0;

- *val = simple_strtoull(buf, &end, 10);
- if (*end && *end != '\n')
- return -EINVAL;
-
- return 0;
+ return kstrtoull(buf, 10, val);
}

/* ****************************************************************** */
--
2.25.1

2021-05-26 09:14:07

by Chen Huang

Subject: [PATCH -next 2/3] xen: balloon: Replaced simple_strtoull() with kstrtoull()

The simple_strtoull() function is deprecated in some situation, since
it does not check for the range overflow, use kstrtoull() instead.

Signed-off-by: Chen Huang <[email protected]>
---
drivers/xen/xen-balloon.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c
index a8d24433c8e9..1fba838963d2 100644
--- a/drivers/xen/xen-balloon.c
+++ b/drivers/xen/xen-balloon.c
@@ -163,13 +163,16 @@ static ssize_t store_target_kb(struct device *dev,
const char *buf,
size_t count)
{
- char *endchar;
+ ssize_t ret;
unsigned long long target_bytes;

if (!capable(CAP_SYS_ADMIN))
return -EPERM;

- target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
+ ret = kstrtoull(buf, 0, &target_bytes);
+ if (ret)
+ return ret;
+ target_bytes *= 1024;

balloon_set_new_target(target_bytes >> PAGE_SHIFT);

--
2.25.1

2021-05-26 09:16:14

by Chen Huang

Subject: [PATCH -next 3/3] ocfs2: Replaced simple_strtoull() with kstrtoull()

The simple_strtoull() function is deprecated in some situation since
it does not check for the range overflow, use kstrtoull() instead.

Signed-off-by: Chen Huang <[email protected]>
---
fs/ocfs2/cluster/heartbeat.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index 1169c8dc9106..f89ffcbd585f 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1596,12 +1596,13 @@ static ssize_t o2hb_region_start_block_store(struct config_item *item,
struct o2hb_region *reg = to_o2hb_region(item);
unsigned long long tmp;
char *p = (char *)page;
+ ssize_t ret;

if (reg->hr_bdev)
return -EINVAL;

- tmp = simple_strtoull(p, &p, 0);
- if (!p || (*p && (*p != '\n')))
+ ret = kstrtoull(p, 0, &tmp);
+ if (ret)
return -EINVAL;

reg->hr_start_block = tmp;
--
2.25.1

2021-05-27 20:53:08

by David Laight

Subject: RE: [PATCH -next 2/3] xen: balloon: Replaced simple_strtoull() with kstrtoull()

From: Chen Huang
> Sent: 26 May 2021 10:20
>
> The simple_strtoull() function is deprecated in some situation, since
> it does not check for the range overflow, use kstrtoull() instead.
>
...
> - target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
> + ret = kstrtoull(buf, 0, &target_bytes);
> + if (ret)
> + return ret;
> + target_bytes *= 1024;

I'd have thought it was more important to check *endchar
than overflow.
If you are worried about overflow you need a range check
before the multiply.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2021-05-27 21:20:30

by Dan Carpenter

Subject: Re: [PATCH -next 2/3] xen: balloon: Replaced simple_strtoull() with kstrtoull()

On Thu, May 27, 2021 at 02:10:21PM +0000, David Laight wrote:
> From: Chen Huang
> > Sent: 26 May 2021 10:20
> >
> > The simple_strtoull() function is deprecated in some situation, since
> > it does not check for the range overflow, use kstrtoull() instead.
> >
> ...
> > - target_bytes = simple_strtoull(buf, &endchar, 0) * 1024;
> > + ret = kstrtoull(buf, 0, &target_bytes);
> > + if (ret)
> > + return ret;
> > + target_bytes *= 1024;
>
> I'd have thought it was more important to check *endchar
> than overflow.

That's one of the differences between simple_strtoull() and kstrtoull().
The simple_strtoull() will accept a string like "123ABC", but kstrtoull()
will only accept NUL terminated numbers or a newline followed by a NUL
terminator. Which is fine in this context because users will be doing
"echo 1234 > /sys/foo".

> If you are worried about overflow you need a range check
> before the multiply.

This is probably a case where if the users cause an integer overflow
then they get what they deserve.

regards,
dan carpenter