xfs: factor out a helper for a single XFS_IOC_ATTRMULTI_BY_HANDLE op

Add a new helper to handle a single attr multi ioctl operation that
can be shared between the native and compat ioctl implementation.

There is a slight change in behaviour in that we don't break out of the
loop when copying in the attribute name fails.  The previous behaviour
was rather inconsistent here as it continued for any other kind of
error, and that we don't clear the flags in the structure returned
to userspace, a behavior only introduced as a bug fix in the last
merge window.

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 df658eacbe58049134a18112392457fdd84f4557..d42be87ca143d16959dc84944c0f6e8b5d58e9e2 100644 (file)
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -349,7 +349,7 @@ out_dput:
        return error;
 }
 
-int
+static int
 xfs_attrmulti_attr_get(
        struct inode            *inode,
        unsigned char           *name,
@@ -381,7 +381,7 @@ out_kfree:
        return error;
 }
 
-int
+static int
 xfs_attrmulti_attr_set(
        struct inode            *inode,
        unsigned char           *name,
@@ -412,6 +412,51 @@ xfs_attrmulti_attr_set(
        return error;
 }
 
+int
+xfs_ioc_attrmulti_one(
+       struct file             *parfilp,
+       struct inode            *inode,
+       uint32_t                opcode,
+       void __user             *uname,
+       void __user             *value,
+       uint32_t                *len,
+       uint32_t                flags)
+{
+       unsigned char           *name;
+       int                     error;
+
+       if ((flags & ATTR_ROOT) && (flags & ATTR_SECURE))
+               return -EINVAL;
+       flags &= ~ATTR_KERNEL_FLAGS;
+
+       name = strndup_user(uname, MAXNAMELEN);
+       if (IS_ERR(name))
+               return PTR_ERR(name);
+
+       switch (opcode) {
+       case ATTR_OP_GET:
+               error = xfs_attrmulti_attr_get(inode, name, value, len, flags);
+               break;
+       case ATTR_OP_REMOVE:
+               value = NULL;
+               *len = 0;
+               /* fall through */
+       case ATTR_OP_SET:
+               error = mnt_want_write_file(parfilp);
+               if (error)
+                       break;
+               error = xfs_attrmulti_attr_set(inode, name, value, *len, flags);
+               mnt_drop_write_file(parfilp);
+               break;
+       default:
+               error = -EINVAL;
+               break;
+       }
+
+       kfree(name);
+       return error;
+}
+
 STATIC int
 xfs_attrmulti_by_handle(
        struct file             *parfilp,
@@ -422,7 +467,6 @@ xfs_attrmulti_by_handle(
        xfs_fsop_attrmulti_handlereq_t am_hreq;
        struct dentry           *dentry;
        unsigned int            i, size;
-       unsigned char           *attr_name;
 
        if (!capable(CAP_SYS_ADMIN))
                return -EPERM;
@@ -450,49 +494,10 @@ xfs_attrmulti_by_handle(
 
        error = 0;
        for (i = 0; i < am_hreq.opcount; i++) {
-               if ((ops[i].am_flags & ATTR_ROOT) &&
-                   (ops[i].am_flags & ATTR_SECURE)) {
-                       ops[i].am_error = -EINVAL;
-                       continue;
-               }
-               ops[i].am_flags &= ~ATTR_KERNEL_FLAGS;
-
-               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:
-                       ops[i].am_error = xfs_attrmulti_attr_get(
-                                       d_inode(dentry), attr_name,
-                                       ops[i].am_attrvalue, &ops[i].am_length,
-                                       ops[i].am_flags);
-                       break;
-               case ATTR_OP_SET:
-                       ops[i].am_error = mnt_want_write_file(parfilp);
-                       if (ops[i].am_error)
-                               break;
-                       ops[i].am_error = xfs_attrmulti_attr_set(
-                                       d_inode(dentry), attr_name,
-                                       ops[i].am_attrvalue, ops[i].am_length,
-                                       ops[i].am_flags);
-                       mnt_drop_write_file(parfilp);
-                       break;
-               case ATTR_OP_REMOVE:
-                       ops[i].am_error = mnt_want_write_file(parfilp);
-                       if (ops[i].am_error)
-                               break;
-                       ops[i].am_error = xfs_attrmulti_attr_set(
-                                       d_inode(dentry), attr_name, NULL, 0,
-                                       ops[i].am_flags);
-                       mnt_drop_write_file(parfilp);
-                       break;
-               default:
-                       ops[i].am_error = -EINVAL;
-               }
-               kfree(attr_name);
+               ops[i].am_error = xfs_ioc_attrmulti_one(parfilp,
+                               d_inode(dentry), ops[i].am_opcode,
+                               ops[i].am_attrname, ops[i].am_attrvalue,
+                               &ops[i].am_length, ops[i].am_flags);
        }
 
        if (copy_to_user(am_hreq.ops, ops, size))

diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h
index 819504df00ae2b1275d796cc21f197b489018c79..bb50cb3dc61f94b37d1f19f8cf05f21c0bd675a1 100644 (file)
--- a/fs/xfs/xfs_ioctl.h
+++ b/fs/xfs/xfs_ioctl.h
@@ -30,21 +30,9 @@ xfs_readlink_by_handle(
        struct file             *parfilp,
        xfs_fsop_handlereq_t    *hreq);
 
-extern int
-xfs_attrmulti_attr_get(
-       struct inode            *inode,
-       unsigned char           *name,
-       unsigned char           __user *ubuf,
-       uint32_t                *len,
-       uint32_t                flags);
-
-extern int
-xfs_attrmulti_attr_set(
-       struct inode            *inode,
-       unsigned char           *name,
-       const unsigned char     __user *ubuf,
-       uint32_t                len,
-       uint32_t                flags);
+int xfs_ioc_attrmulti_one(struct file *parfilp, struct inode *inode,
+               uint32_t opcode, void __user *uname, void __user *value,
+               uint32_t *len, uint32_t flags);
 
 extern struct dentry *
 xfs_handle_to_dentry(

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 936c2f62fb6cf22dfae8a8e49a8c1653fe1686bf..e1daf095c5851cfe0262043bdf577ca1061b52bd 100644 (file)
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -418,7 +418,6 @@ xfs_compat_attrmulti_by_handle(
        compat_xfs_fsop_attrmulti_handlereq_t   am_hreq;
        struct dentry                           *dentry;
        unsigned int                            i, size;
-       unsigned char                           *attr_name;
 
        if (!capable(CAP_SYS_ADMIN))
                return -EPERM;
@@ -447,50 +446,11 @@ xfs_compat_attrmulti_by_handle(
 
        error = 0;
        for (i = 0; i < am_hreq.opcount; i++) {
-               if ((ops[i].am_flags & ATTR_ROOT) &&
-                   (ops[i].am_flags & ATTR_SECURE)) {
-                       ops[i].am_error = -EINVAL;
-                       continue;
-               }
-               ops[i].am_flags &= ~ATTR_KERNEL_FLAGS;
-
-               attr_name = strndup_user(compat_ptr(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:
-                       ops[i].am_error = xfs_attrmulti_attr_get(
-                                       d_inode(dentry), attr_name,
-                                       compat_ptr(ops[i].am_attrvalue),
-                                       &ops[i].am_length, ops[i].am_flags);
-                       break;
-               case ATTR_OP_SET:
-                       ops[i].am_error = mnt_want_write_file(parfilp);
-                       if (ops[i].am_error)
-                               break;
-                       ops[i].am_error = xfs_attrmulti_attr_set(
-                                       d_inode(dentry), attr_name,
-                                       compat_ptr(ops[i].am_attrvalue),
-                                       ops[i].am_length, ops[i].am_flags);
-                       mnt_drop_write_file(parfilp);
-                       break;
-               case ATTR_OP_REMOVE:
-                       ops[i].am_error = mnt_want_write_file(parfilp);
-                       if (ops[i].am_error)
-                               break;
-                       ops[i].am_error = xfs_attrmulti_attr_set(
-                                       d_inode(dentry), attr_name, NULL, 0,
-                                       ops[i].am_flags);
-                       mnt_drop_write_file(parfilp);
-                       break;
-               default:
-                       ops[i].am_error = -EINVAL;
-               }
-               kfree(attr_name);
+               ops[i].am_error = xfs_ioc_attrmulti_one(parfilp,
+                               d_inode(dentry), ops[i].am_opcode,
+                               compat_ptr(ops[i].am_attrname),
+                               compat_ptr(ops[i].am_attrvalue),
+                               &ops[i].am_length, ops[i].am_flags);
        }
 
        if (copy_to_user(compat_ptr(am_hreq.ops), ops, size))