Simplify the user copy code by using strndup_user. This means that we now do one memory allocation per operation instead of one per ioctl, but memory allocations are cheap compared to the actual file system operations. Signed-off-by: Christoph Hellwig <hch@xxxxxx> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --- fs/xfs/xfs_ioctl.c | 17 +++++------------ fs/xfs/xfs_ioctl32.c | 17 +++++------------ 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index b806003caacd..bb490a954c0b 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -448,11 +448,6 @@ xfs_attrmulti_by_handle( goto out_dput; } - error = -ENOMEM; - attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL); - if (!attr_name) - goto out_kfree_ops; - error = 0; for (i = 0; i < am_hreq.opcount; i++) { if ((ops[i].am_flags & ATTR_ROOT) && @@ -462,12 +457,11 @@ xfs_attrmulti_by_handle( } ops[i].am_flags &= ~ATTR_KERNEL_FLAGS; - ops[i].am_error = strncpy_from_user((char *)attr_name, - ops[i].am_attrname, MAXNAMELEN); - if (ops[i].am_error == 0 || ops[i].am_error == MAXNAMELEN) - error = -ERANGE; - if (ops[i].am_error < 0) + attr_name = strndup_user(ops[i].am_attrname, MAXNAMELEN); + if (IS_ERR(attr_name)) { + ops[i].am_error = PTR_ERR(attr_name); break; + } switch (ops[i].am_opcode) { case ATTR_OP_GET: @@ -498,13 +492,12 @@ xfs_attrmulti_by_handle( default: ops[i].am_error = -EINVAL; } + kfree(attr_name); } if (copy_to_user(am_hreq.ops, ops, size)) error = -EFAULT; - kfree(attr_name); - out_kfree_ops: kfree(ops); out_dput: dput(dentry); diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c index e085f304e539..936c2f62fb6c 100644 --- a/fs/xfs/xfs_ioctl32.c +++ b/fs/xfs/xfs_ioctl32.c @@ -445,11 +445,6 @@ xfs_compat_attrmulti_by_handle( goto out_dput; } - error = -ENOMEM; - attr_name = kmalloc(MAXNAMELEN, GFP_KERNEL); - if (!attr_name) - goto out_kfree_ops; - error = 0; for (i = 0; i < am_hreq.opcount; i++) { if ((ops[i].am_flags & ATTR_ROOT) && @@ -459,13 +454,12 @@ xfs_compat_attrmulti_by_handle( } ops[i].am_flags &= ~ATTR_KERNEL_FLAGS; - ops[i].am_error = strncpy_from_user((char *)attr_name, - compat_ptr(ops[i].am_attrname), + attr_name = strndup_user(compat_ptr(ops[i].am_attrname), MAXNAMELEN); - if (ops[i].am_error == 0 || ops[i].am_error == MAXNAMELEN) - error = -ERANGE; - if (ops[i].am_error < 0) + if (IS_ERR(attr_name)) { + ops[i].am_error = PTR_ERR(attr_name); break; + } switch (ops[i].am_opcode) { case ATTR_OP_GET: @@ -496,13 +490,12 @@ xfs_compat_attrmulti_by_handle( default: ops[i].am_error = -EINVAL; } + kfree(attr_name); } if (copy_to_user(compat_ptr(am_hreq.ops), ops, size)) error = -EFAULT; - kfree(attr_name); - out_kfree_ops: kfree(ops); out_dput: dput(dentry); -- 2.24.1