Remove open-coded memdup_user() in fs/.
text data bss dec hex filename
7684360 520520 5629456 13834336 d31860 vmlinux.o.orig
text data bss dec hex filename
7683392 520520 5629456 13833368 d31498 vmlinux.o
fs/btrfs/ioctl.c | 49 +++++++++------------------------------
fs/btrfs/super.c | 13 +++-------
fs/ecryptfs/miscdev.c | 15 ++++--------
fs/ncpfs/ioctl.c | 21 +++++-----------
fs/sysfs/bin.c | 13 ++--------
fs/xattr.c | 10 ++-----
fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++-------------
fs/xfs/linux-2.6/xfs_ioctl32.c | 12 +++------
8 files changed, 45 insertions(+), 111 deletions(-)
---
Remove open-coded memdup_user()
Signed-off-by: Li Zefan <[email protected]>
---
fs/xattr.c | 10 +++-------
1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index 197c4fc..d51b8f9 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -237,13 +237,9 @@ setxattr(struct dentry *d, const char __user *name, const void __user *value,
if (size) {
if (size > XATTR_SIZE_MAX)
return -E2BIG;
- kvalue = kmalloc(size, GFP_KERNEL);
- if (!kvalue)
- return -ENOMEM;
- if (copy_from_user(kvalue, value, size)) {
- kfree(kvalue);
- return -EFAULT;
- }
+ kvalue = memdup_user(value, size);
+ if (IS_ERR(kvalue))
+ return PTR_ERR(kvalue);
}
error = vfs_setxattr(d, kname, kvalue, size, flags);
--
1.5.4.rc3
Remove open-coded memdup_user().
Note this changes some GFP_NOFS to GFP_KERNEL, since copy_from_user() may
cause pagefault, it's pointless to pass GFP_NOFS to kmalloc().
Signed-off-by: Li Zefan <[email protected]>
---
fs/btrfs/ioctl.c | 49 ++++++++++++-------------------------------------
fs/btrfs/super.c | 13 ++++---------
2 files changed, 16 insertions(+), 46 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 7594bec..9f135e8 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -461,15 +461,9 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- vol_args = kmalloc(sizeof(*vol_args), GFP_NOFS);
-
- if (!vol_args)
- return -ENOMEM;
-
- if (copy_from_user(vol_args, arg, sizeof(*vol_args))) {
- ret = -EFAULT;
- goto out;
- }
+ vol_args = memdup_user(arg, sizeof(*vol_args));
+ if (IS_ERR(vol_args))
+ return PTR_ERR(vol_args);
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
namelen = strlen(vol_args->name);
@@ -545,7 +539,6 @@ static int btrfs_ioctl_resize(struct btrfs_root *root, void __user *arg)
out_unlock:
mutex_unlock(&root->fs_info->volume_mutex);
-out:
kfree(vol_args);
return ret;
}
@@ -565,15 +558,9 @@ static noinline int btrfs_ioctl_snap_create(struct file *file,
if (root->fs_info->sb->s_flags & MS_RDONLY)
return -EROFS;
- vol_args = kmalloc(sizeof(*vol_args), GFP_NOFS);
-
- if (!vol_args)
- return -ENOMEM;
-
- if (copy_from_user(vol_args, arg, sizeof(*vol_args))) {
- ret = -EFAULT;
- goto out;
- }
+ vol_args = memdup_user(arg, sizeof(*vol_args));
+ if (IS_ERR(vol_args))
+ return PTR_ERR(vol_args);
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
namelen = strlen(vol_args->name);
@@ -675,19 +662,13 @@ static long btrfs_ioctl_add_dev(struct btrfs_root *root, void __user *arg)
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- vol_args = kmalloc(sizeof(*vol_args), GFP_NOFS);
+ vol_args = memdup_user(arg, sizeof(*vol_args));
+ if (IS_ERR(vol_args))
+ return PTR_ERR(vol_args);
- if (!vol_args)
- return -ENOMEM;
-
- if (copy_from_user(vol_args, arg, sizeof(*vol_args))) {
- ret = -EFAULT;
- goto out;
- }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
ret = btrfs_init_new_device(root, vol_args->name);
-out:
kfree(vol_args);
return ret;
}
@@ -703,19 +684,13 @@ static long btrfs_ioctl_rm_dev(struct btrfs_root *root, void __user *arg)
if (root->fs_info->sb->s_flags & MS_RDONLY)
return -EROFS;
- vol_args = kmalloc(sizeof(*vol_args), GFP_NOFS);
+ vol_args = memdup_user(arg, sizeof(*vol_args));
+ if (IS_ERR(vol_args))
+ return PTR_ERR(vol_args);
- if (!vol_args)
- return -ENOMEM;
-
- if (copy_from_user(vol_args, arg, sizeof(*vol_args))) {
- ret = -EFAULT;
- goto out;
- }
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
ret = btrfs_rm_device(root, vol_args->name);
-out:
kfree(vol_args);
return ret;
}
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 9744af9..a7acfe6 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -635,14 +635,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- vol = kmalloc(sizeof(*vol), GFP_KERNEL);
- if (!vol)
- return -ENOMEM;
-
- if (copy_from_user(vol, (void __user *)arg, sizeof(*vol))) {
- ret = -EFAULT;
- goto out;
- }
+ vol = memdup_user((void __user *)arg, sizeof(*vol));
+ if (IS_ERR(vol))
+ return PTR_ERR(vol);
switch (cmd) {
case BTRFS_IOC_SCAN_DEV:
@@ -650,7 +645,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
&btrfs_fs_type, &fs_devices);
break;
}
-out:
+
kfree(vol);
return ret;
}
--
1.5.4.rc3
Remove open-coded memdup_user().
Signed-off-by: Li Zefan <[email protected]>
---
fs/sysfs/bin.c | 13 +++----------
1 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 93e0c02..9345806 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -157,14 +157,9 @@ static ssize_t write(struct file *file, const char __user *userbuf,
count = size - offs;
}
- temp = kmalloc(count, GFP_KERNEL);
- if (!temp)
- return -ENOMEM;
-
- if (copy_from_user(temp, userbuf, count)) {
- count = -EFAULT;
- goto out_free;
- }
+ temp = memdup_user(userbuf, count);
+ if (IS_ERR(temp))
+ return PTR_ERR(temp);
mutex_lock(&bb->mutex);
@@ -176,8 +171,6 @@ static ssize_t write(struct file *file, const char __user *userbuf,
if (count > 0)
*off = offs + count;
-out_free:
- kfree(temp);
return count;
}
--
1.5.4.rc3
Remove open-coded memdup_user()
Signed-off-by: Li Zefan <[email protected]>
---
fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++----------------
fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++--------
2 files changed, 11 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c
index d0b4994..34eaab6 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl.c
@@ -489,17 +489,12 @@ xfs_attrmulti_attr_set(
if (len > XATTR_SIZE_MAX)
return EINVAL;
- kbuf = kmalloc(len, GFP_KERNEL);
- if (!kbuf)
- return ENOMEM;
-
- if (copy_from_user(kbuf, ubuf, len))
- goto out_kfree;
+ kbuf = memdup_user(ubuf, len);
+ if (IS_ERR(kbuf))
+ return PTR_ERR(kbuf);
error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);
- out_kfree:
- kfree(kbuf);
return error;
}
@@ -540,20 +535,16 @@ xfs_attrmulti_by_handle(
if (!size || size > 16 * PAGE_SIZE)
goto out_dput;
- error = ENOMEM;
- ops = kmalloc(size, GFP_KERNEL);
- if (!ops)
+ ops = memdup_user(am_hreq.ops, size);
+ if (IS_ERR(ops)) {
+ error = PTR_ERR(ops);
goto out_dput;
-
- error = EFAULT;
- if (copy_from_user(ops, am_hreq.ops, size))
- goto out_kfree_ops;
+ }
attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL);
if (!attr_name)
goto out_kfree_ops;
-
error = 0;
for (i = 0; i < am_hreq.opcount; i++) {
ops[i].am_error = strncpy_from_user(attr_name,
diff --git a/fs/xfs/linux-2.6/xfs_ioctl32.c b/fs/xfs/linux-2.6/xfs_ioctl32.c
index c70c4e3..0882d16 100644
--- a/fs/xfs/linux-2.6/xfs_ioctl32.c
+++ b/fs/xfs/linux-2.6/xfs_ioctl32.c
@@ -427,20 +427,16 @@ xfs_compat_attrmulti_by_handle(
if (!size || size > 16 * PAGE_SIZE)
goto out_dput;
- error = ENOMEM;
- ops = kmalloc(size, GFP_KERNEL);
- if (!ops)
+ ops = memdup_user(compat_ptr(am_hreq.ops), size);
+ if (IS_ERR(ops)) {
+ error = PTR_ERR(ops);
goto out_dput;
-
- error = EFAULT;
- if (copy_from_user(ops, compat_ptr(am_hreq.ops), size))
- goto out_kfree_ops;
+ }
attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL);
if (!attr_name)
goto out_kfree_ops;
-
error = 0;
for (i = 0; i < am_hreq.opcount; i++) {
ops[i].am_error = strncpy_from_user(attr_name,
--
1.5.4.rc3
Remove open-coded memdup_user()
Signed-off-by: Li Zefan <[email protected]>
---
fs/ncpfs/ioctl.c | 21 +++++++--------------
1 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/fs/ncpfs/ioctl.c b/fs/ncpfs/ioctl.c
index f54360f..fa038df 100644
--- a/fs/ncpfs/ioctl.c
+++ b/fs/ncpfs/ioctl.c
@@ -660,13 +660,10 @@ outrel:
if (user.object_name_len > NCP_OBJECT_NAME_MAX_LEN)
return -ENOMEM;
if (user.object_name_len) {
- newname = kmalloc(user.object_name_len, GFP_USER);
- if (!newname)
- return -ENOMEM;
- if (copy_from_user(newname, user.object_name, user.object_name_len)) {
- kfree(newname);
- return -EFAULT;
- }
+ newname = memdup_user(user.object_name,
+ user.object_name_len);
+ if (IS_ERR(newname))
+ return PTR_ERR(newname);
} else {
newname = NULL;
}
@@ -760,13 +757,9 @@ outrel:
if (user.len > NCP_PRIVATE_DATA_MAX_LEN)
return -ENOMEM;
if (user.len) {
- new = kmalloc(user.len, GFP_USER);
- if (!new)
- return -ENOMEM;
- if (copy_from_user(new, user.data, user.len)) {
- kfree(new);
- return -EFAULT;
- }
+ new = memdup_user(user.data, user.len);
+ if (IS_ERR(new))
+ return PTR_ERR(new);
} else {
new = NULL;
}
--
1.5.4.rc3
Remove open-coded memdup_user().
Signed-off-by: Li Zefan <[email protected]>
---
fs/ecryptfs/miscdev.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/fs/ecryptfs/miscdev.c b/fs/ecryptfs/miscdev.c
index a67fea6..dda3c58 100644
--- a/fs/ecryptfs/miscdev.c
+++ b/fs/ecryptfs/miscdev.c
@@ -418,18 +418,13 @@ ecryptfs_miscdev_write(struct file *file, const char __user *buf,
if (count == 0)
goto out;
- data = kmalloc(count, GFP_KERNEL);
- if (!data) {
- printk(KERN_ERR "%s: Out of memory whilst attempting to "
- "kmalloc([%zd], GFP_KERNEL)\n", __func__, count);
+
+ data = memdup_user(buf, count);
+ if (IS_ERR(data)) {
+ printk(KERN_ERR "%s: memdup_user returned error [%ld]\n",
+ __func__, PTR_ERR(data));
goto out;
}
- rc = copy_from_user(data, buf, count);
- if (rc) {
- printk(KERN_ERR "%s: copy_from_user returned error [%d]\n",
- __func__, rc);
- goto out_free;
- }
sz = count;
i = 0;
switch (data[i++]) {
--
1.5.4.rc3
On Wed, Apr 08, 2009 at 03:08:04PM +0800, Li Zefan wrote:
> Remove open-coded memdup_user()
>
> Signed-off-by: Li Zefan <[email protected]>
> ---
> fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++----------------
> fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++--------
> 2 files changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c
> index d0b4994..34eaab6 100644
> --- a/fs/xfs/linux-2.6/xfs_ioctl.c
> +++ b/fs/xfs/linux-2.6/xfs_ioctl.c
> @@ -489,17 +489,12 @@ xfs_attrmulti_attr_set(
> if (len > XATTR_SIZE_MAX)
> return EINVAL;
>
> - kbuf = kmalloc(len, GFP_KERNEL);
> - if (!kbuf)
> - return ENOMEM;
> -
> - if (copy_from_user(kbuf, ubuf, len))
> - goto out_kfree;
> + kbuf = memdup_user(ubuf, len);
> + if (IS_ERR(kbuf))
> + return PTR_ERR(kbuf);
wouldn't NULL be a better error return for this kind of interface,
matching kmalloc?
Otherwise the patch looks good to me.
Reviewed-by: Christoph Hellwig <[email protected]>
> wouldn't NULL be a better error return for this kind of interface,
> matching kmalloc?
I guess returning an error code from memdup_user() lets callers
distinguish between ENOMEM and EFAULT. Not sure if that's important or
not but there probably are at least some sites that care.
- R.
On Apr 8, 2009, at 8:22 AM, Christoph Hellwig wrote:
> On Wed, Apr 08, 2009 at 03:08:04PM +0800, Li Zefan wrote:
>> Remove open-coded memdup_user()
>>
>> Signed-off-by: Li Zefan <[email protected]>
>> ---
>> fs/xfs/linux-2.6/xfs_ioctl.c | 23 +++++++----------------
>> fs/xfs/linux-2.6/xfs_ioctl32.c | 12 ++++--------
>> 2 files changed, 11 insertions(+), 24 deletions(-)
>>
>> diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/
>> xfs_ioctl.c
>> index d0b4994..34eaab6 100644
>> --- a/fs/xfs/linux-2.6/xfs_ioctl.c
>> +++ b/fs/xfs/linux-2.6/xfs_ioctl.c
>> @@ -489,17 +489,12 @@ xfs_attrmulti_attr_set(
>> if (len > XATTR_SIZE_MAX)
>> return EINVAL;
>>
>> - kbuf = kmalloc(len, GFP_KERNEL);
>> - if (!kbuf)
>> - return ENOMEM;
>> -
>> - if (copy_from_user(kbuf, ubuf, len))
>> - goto out_kfree;
>> + kbuf = memdup_user(ubuf, len);
>> + if (IS_ERR(kbuf))
>> + return PTR_ERR(kbuf);
>
> wouldn't NULL be a better error return for this kind of interface,
> matching kmalloc?
>
>
> Otherwise the patch looks good to me.
>
> Reviewed-by: Christoph Hellwig <[email protected]>
Looks good to me too.
Reviewed-by: Felix Blyakher <[email protected]>
p.s. Replying to reply as I couldn't find the original post.
Roland Dreier wrote:
> > wouldn't NULL be a better error return for this kind of interface,
> > matching kmalloc?
>
> I guess returning an error code from memdup_user() lets callers
> distinguish between ENOMEM and EFAULT. Not sure if that's important or
> not but there probably are at least some sites that care.
>
Right, and this API is simlilar to strndup_user().