* [PATCH] mm: Replace-simple_strtoul-with-kstrtoul
[not found] <CGME20171109113212epcas5p4b93d4830869468901f4003bde11e3d16@epcas5p4.samsung.com>
@ 2017-11-09 11:28 ` Manjeet Pawar
2017-11-09 13:04 ` Matthew Wilcox
0 siblings, 1 reply; 2+ messages in thread
From: Manjeet Pawar @ 2017-11-09 11:28 UTC (permalink / raw)
To: linux-kernel, linux-mm, vbabka, mhocko, akpm, hughd
Cc: a.sahrawat, pankaj.m, lalit.mohan, Manjeet Pawar,
Vinay Kumar Rijhwani, Rohit Thapliyal
simple_strtoul() is obselete now, so using newer function kstrtoul()
Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
Signed-off-by: Vinay Kumar Rijhwani <v.rijhwani@samsung.com>
Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
---
mm/page_alloc.c | 3 +--
mm/shmem.c | 11 +++++------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 77e4d3c..f9d812e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7197,9 +7197,8 @@ int percpu_pagelist_fraction_sysctl_handler(struct ctl_table *table, int write,
static int __init set_hashdist(char *str)
{
- if (!str)
+ if (!str || kstrtoul(str, 0, (unsigned long *)&hashdist))
return 0;
- hashdist = simple_strtoul(str, &str, 0);
return 1;
}
__setup("hashdist=", set_hashdist);
diff --git a/mm/shmem.c b/mm/shmem.c
index 07a1d22..bf7d905 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3470,6 +3470,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
struct mempolicy *mpol = NULL;
uid_t uid;
gid_t gid;
+ unsigned long mode;
while (options != NULL) {
this_char = options;
@@ -3522,14 +3523,13 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
} else if (!strcmp(this_char,"mode")) {
if (remount)
continue;
- sbinfo->mode = simple_strtoul(value, &rest, 8) & 07777;
- if (*rest)
+ if (kstrtoul(value, 8, &mode))
goto bad_val;
+ sbinfo->mode = mode & 07777;
} else if (!strcmp(this_char,"uid")) {
if (remount)
continue;
- uid = simple_strtoul(value, &rest, 0);
- if (*rest)
+ if (kstrtoul(value, 0, (unsigned long *)&uid))
goto bad_val;
sbinfo->uid = make_kuid(current_user_ns(), uid);
if (!uid_valid(sbinfo->uid))
@@ -3537,8 +3537,7 @@ static int shmem_parse_options(char *options, struct shmem_sb_info *sbinfo,
} else if (!strcmp(this_char,"gid")) {
if (remount)
continue;
- gid = simple_strtoul(value, &rest, 0);
- if (*rest)
+ if (kstrtoul(value, 0, (unsigned long *)&gid))
goto bad_val;
sbinfo->gid = make_kgid(current_user_ns(), gid);
if (!gid_valid(sbinfo->gid))
--
1.9.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] mm: Replace-simple_strtoul-with-kstrtoul
2017-11-09 11:28 ` [PATCH] mm: Replace-simple_strtoul-with-kstrtoul Manjeet Pawar
@ 2017-11-09 13:04 ` Matthew Wilcox
0 siblings, 0 replies; 2+ messages in thread
From: Matthew Wilcox @ 2017-11-09 13:04 UTC (permalink / raw)
To: Manjeet Pawar
Cc: linux-kernel, linux-mm, vbabka, mhocko, akpm, hughd, a.sahrawat,
pankaj.m, lalit.mohan, Vinay Kumar Rijhwani, Rohit Thapliyal
On Thu, Nov 09, 2017 at 04:58:18PM +0530, Manjeet Pawar wrote:
> simple_strtoul() is obselete now, so using newer function kstrtoul()
>
> Signed-off-by: Manjeet Pawar <manjeet.p@samsung.com>
> Signed-off-by: Vinay Kumar Rijhwani <v.rijhwani@samsung.com>
> Signed-off-by: Rohit Thapliyal <r.thapliyal@samsung.com>
NAK NAK NAK.
You haven't tested this on a 64-bit big-endian machine.
> static int __init set_hashdist(char *str)
> {
> - if (!str)
> + if (!str || kstrtoul(str, 0, (unsigned long *)&hashdist))
> return 0;
> - hashdist = simple_strtoul(str, &str, 0);
> return 1;
The context missing from this patch is:
int hashdist = HASHDIST_DEFAULT;
So you're taking the address of an int and passing it to a function
which is expecting a pointer to an unsigned long. That works on a
32-bit machine because ints and longs are the same size. On a 64-bit
little-endian machine, the bits are in the right place, but kstrtoul()
will overwrite the 32 bits after the int with zeroes. On a 64-bit
big-endian machine, you'll overwrite the int that you're pointing to
with zeroes and the 32 bits after the int will have the data you're
looking for.
There's a kstrtoint(). Why would you not just use that?
Also, I'm shocked that this went through a chain of three different
sign-offs with nobody noticing the problem. Do none of you understand C?
(similar problems snipped).
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-11-09 13:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20171109113212epcas5p4b93d4830869468901f4003bde11e3d16@epcas5p4.samsung.com>
2017-11-09 11:28 ` [PATCH] mm: Replace-simple_strtoul-with-kstrtoul Manjeet Pawar
2017-11-09 13:04 ` Matthew Wilcox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).