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>

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index b3cd6e8ad59d1ea91fe7dd03ac7e63e6d036bfdf..74ee0f8ef8ddee1472567930c116787915ab3ded 100644 (file)
--- 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 b6c44e75b361f5b1cda081fe9bcdc3bd8ff600de..d12bcda911e143ca778d0bae4e931e5bb7f8e74e 100644 (file)
--- 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 6cf7d161baa1ecd3d069ecf1d30b214e8d6f8c37..d3e9e3fe0b587ae0176363a88cdf7ecaa9162b07 100644 (file)
--- 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);
 }
 
 /*