xfs: use strndup_user in XFS_IOC_ATTRMULTI_BY_HANDLE

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.  Also the error for an invalid path is now EINVAL or EFAULT
instead of the previous odd and undocumented ERANGE.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index ef050894526794543e82ac6bc10466c9fea31f9f..df658eacbe58049134a18112392457fdd84f4557 100644 (file)
--- 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 e085f304e5395d1397a6a2c71d68c9627a67860d..936c2f62fb6cf22dfae8a8e49a8c1653fe1686bf 100644 (file)
--- 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);