afs: Fix afs_xattr_get_yfs() to not try freeing an error value

afs_xattr_get_yfs() tries to free yacl, which may hold an error value (say
if yfs_fs_fetch_opaque_acl() failed and returned an error).

Fix this by allocating yacl up front (since it's a fixed-length struct,
unlike afs_acl) and passing it in to the RPC function.  This also allows
the flags to be placed in the object rather than passing them through to
the RPC function.

Fixes: ae46578b963f ("afs: Get YFS ACLs and information through xattrs")
Signed-off-by: David Howells <dhowells@redhat.com>

3 files changed, 53 insertions, 64 deletions

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index b3cd6e8ad59d..74ee0f8ef8dd 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1382,7 +1382,7 @@ struct yfs_acl {
 };
 
 extern void yfs_free_opaque_acl(struct yfs_acl *);
-extern struct yfs_acl *yfs_fs_fetch_opaque_acl(struct afs_fs_cursor *, unsigned int);
+extern struct yfs_acl *yfs_fs_fetch_opaque_acl(struct afs_fs_cursor *, struct yfs_acl *);
 extern int yfs_fs_store_opaque_acl2(struct afs_fs_cursor *, const struct afs_acl *);
 
 /*
diff --git a/fs/afs/xattr.c b/fs/afs/xattr.c
index b6c44e75b361..d12bcda911e1 100644
--- a/fs/afs/xattr.c
+++ b/fs/afs/xattr.c
@@ -148,9 +148,8 @@ static int afs_xattr_get_yfs(const struct xattr_handler *handler,
         struct afs_vnode *vnode = AFS_FS_I(inode);
         struct yfs_acl *yacl = NULL;
         struct key *key;
-        unsigned int flags = 0;
         char buf[16], *data;
-        int which = 0, dsize, ret;
+        int which = 0, dsize, ret = -ENOMEM;
 
         if (strcmp(name, "acl") == 0)
                 which = 0;
@@ -163,20 +162,26 @@ static int afs_xattr_get_yfs(const struct xattr_handler *handler,
         else
                 return -EOPNOTSUPP;
 
+        yacl = kzalloc(sizeof(struct yfs_acl), GFP_KERNEL);
+        if (!yacl)
+                goto error;
+
         if (which == 0)
-                flags |= YFS_ACL_WANT_ACL;
+                yacl->flags |= YFS_ACL_WANT_ACL;
         else if (which == 3)
-                flags |= YFS_ACL_WANT_VOL_ACL;
+                yacl->flags |= YFS_ACL_WANT_VOL_ACL;
 
         key = afs_request_key(vnode->volume->cell);
-        if (IS_ERR(key))
-                return PTR_ERR(key);
+        if (IS_ERR(key)) {
+                ret = PTR_ERR(key);
+                goto error_yacl;
+        }
 
         ret = -ERESTARTSYS;
         if (afs_begin_vnode_operation(&fc, vnode, key)) {
                 while (afs_select_fileserver(&fc)) {
                         fc.cb_break = afs_calc_vnode_cb_break(vnode);
-                        yacl = yfs_fs_fetch_opaque_acl(&fc, flags);
+                        yfs_fs_fetch_opaque_acl(&fc, yacl);
                 }
 
                 afs_check_for_remote_deletion(&fc, fc.vnode);
@@ -184,44 +189,45 @@ static int afs_xattr_get_yfs(const struct xattr_handler *handler,
                 ret = afs_end_vnode_operation(&fc);
         }
 
-        if (ret == 0) {
-                switch (which) {
-                case 0:
-                        data = yacl->acl->data;
-                        dsize = yacl->acl->size;
-                        break;
-                case 1:
-                        data = buf;
-                        dsize = snprintf(buf, sizeof(buf), "%u",
-                                         yacl->inherit_flag);
-                        break;
-                case 2:
-                        data = buf;
-                        dsize = snprintf(buf, sizeof(buf), "%u",
-                                         yacl->num_cleaned);
-                        break;
-                case 3:
-                        data = yacl->vol_acl->data;
-                        dsize = yacl->vol_acl->size;
-                        break;
-                default:
-                        ret = -EOPNOTSUPP;
-                        goto out;
-                }
+        if (ret < 0)
+                goto error_key;
+
+        switch (which) {
+        case 0:
+                data = yacl->acl->data;
+                dsize = yacl->acl->size;
+                break;
+        case 1:
+                data = buf;
+                dsize = snprintf(buf, sizeof(buf), "%u", yacl->inherit_flag);
+                break;
+        case 2:
+                data = buf;
+                dsize = snprintf(buf, sizeof(buf), "%u", yacl->num_cleaned);
+                break;
+        case 3:
+                data = yacl->vol_acl->data;
+                dsize = yacl->vol_acl->size;
+                break;
+        default:
+                ret = -EOPNOTSUPP;
+                goto error_key;
+        }
 
-                ret = dsize;
-                if (size > 0) {
-                        if (dsize > size) {
-                                ret = -ERANGE;
-                                goto out;
-                        }
-                        memcpy(buffer, data, dsize);
+        ret = dsize;
+        if (size > 0) {
+                if (dsize > size) {
+                        ret = -ERANGE;
+                        goto error_key;
                 }
+                memcpy(buffer, data, dsize);
         }
 
-out:
-        yfs_free_opaque_acl(yacl);
+error_key:
         key_put(key);
+error_yacl:
+        yfs_free_opaque_acl(yacl);
+error:
         return ret;
 }
 
diff --git a/fs/afs/yfsclient.c b/fs/afs/yfsclient.c
index 6cf7d161baa1..d3e9e3fe0b58 100644
--- a/fs/afs/yfsclient.c
+++ b/fs/afs/yfsclient.c
@@ -2333,12 +2333,6 @@ void yfs_free_opaque_acl(struct yfs_acl *yacl)
         }
 }
 
-static void yfs_destroy_fs_fetch_opaque_acl(struct afs_call *call)
-{
-        yfs_free_opaque_acl(call->reply[0]);
-        afs_flat_call_destructor(call);
-}
-
 /*
  * YFS.FetchOpaqueACL operation type
  */
@@ -2346,18 +2340,17 @@ static const struct afs_call_type yfs_RXYFSFetchOpaqueACL = {
         .name           = "YFS.FetchOpaqueACL",
         .op             = yfs_FS_FetchOpaqueACL,
         .deliver        = yfs_deliver_fs_fetch_opaque_acl,
-        .destructor     = yfs_destroy_fs_fetch_opaque_acl,
+        .destructor     = afs_flat_call_destructor,
 };
 
 /*
  * Fetch the YFS advanced ACLs for a file.
  */
 struct yfs_acl *yfs_fs_fetch_opaque_acl(struct afs_fs_cursor *fc,
-                                        unsigned int flags)
+                                        struct yfs_acl *yacl)
 {
         struct afs_vnode *vnode = fc->vnode;
         struct afs_call *call;
-        struct yfs_acl *yacl;
         struct afs_net *net = afs_v2net(vnode);
         __be32 *bp;
 
@@ -2370,19 +2363,15 @@ struct yfs_acl *yfs_fs_fetch_opaque_acl(struct afs_fs_cursor *fc,
                                    sizeof(__be32) * 2 +
                                    sizeof(struct yfs_xdr_YFSFetchStatus) +
                                    sizeof(struct yfs_xdr_YFSVolSync));
-        if (!call)
-                goto nomem;
-
-        yacl = kzalloc(sizeof(struct yfs_acl), GFP_KERNEL);
-        if (!yacl)
-                goto nomem_call;
+        if (!call) {
+                fc->ac.error = -ENOMEM;
+                return ERR_PTR(-ENOMEM);
+        }
 
-        yacl->flags = flags;
         call->key = fc->key;
         call->reply[0] = yacl;
         call->reply[1] = vnode;
         call->reply[2] = NULL; /* volsync */
-        call->ret_reply0 = true;
 
         /* marshall the parameters */
         bp = call->request;
@@ -2396,12 +2385,6 @@ struct yfs_acl *yfs_fs_fetch_opaque_acl(struct afs_fs_cursor *fc,
         trace_afs_make_fs_call(call, &vnode->fid);
         afs_make_call(&fc->ac, call, GFP_KERNEL);
         return (struct yfs_acl *)afs_wait_for_call_to_complete(call, &fc->ac);
-
-nomem_call:
-        afs_put_call(call);
-nomem:
-        fc->ac.error = -ENOMEM;
-        return ERR_PTR(-ENOMEM);
 }
 
 /*