NFSv4: Fix free of uninitialized nfs4_label on referral lookup.
authorBenjamin Coddington <bcodding@redhat.com>
Sat, 14 May 2022 11:05:13 +0000 (07:05 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 9 Jun 2022 08:30:39 +0000 (10:30 +0200)
[ Upstream commit c3ed222745d9ad7b69299b349a64ba533c64a34f ]

Send along the already-allocated fattr along with nfs4_fs_locations, and
drop the memcpy of fattr.  We end up growing two more allocations, but this
fixes up a crash as:

PID: 790    TASK: ffff88811b43c000  CPU: 0   COMMAND: "ls"
 #0 [ffffc90000857920] panic at ffffffff81b9bfde
 #1 [ffffc900008579c0] do_trap at ffffffff81023a9b
 #2 [ffffc90000857a10] do_error_trap at ffffffff81023b78
 #3 [ffffc90000857a58] exc_stack_segment at ffffffff81be1f45
 #4 [ffffc90000857a80] asm_exc_stack_segment at ffffffff81c009de
 #5 [ffffc90000857b08] nfs_lookup at ffffffffa0302322 [nfs]
 #6 [ffffc90000857b70] __lookup_slow at ffffffff813a4a5f
 #7 [ffffc90000857c60] walk_component at ffffffff813a86c4
 #8 [ffffc90000857cb8] path_lookupat at ffffffff813a9553
 #9 [ffffc90000857cf0] filename_lookup at ffffffff813ab86b

Suggested-by: Trond Myklebust <trondmy@hammerspace.com>
Fixes: 9558a007dbc3 ("NFS: Remove the label from the nfs4_lookup_res struct")
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
fs/nfs/nfs4namespace.c
fs/nfs/nfs4proc.c
fs/nfs/nfs4state.c
fs/nfs/nfs4xdr.c
include/linux/nfs_xdr.h

index 3680c8da510c9787bcbae896a292a2cfec039469..f2dbf904c59895b7e6c4f13fd595a50c98164657 100644 (file)
@@ -417,6 +417,9 @@ static int nfs_do_refmount(struct fs_context *fc, struct rpc_clnt *client)
        fs_locations = kmalloc(sizeof(struct nfs4_fs_locations), GFP_KERNEL);
        if (!fs_locations)
                goto out_free;
+       fs_locations->fattr = nfs_alloc_fattr();
+       if (!fs_locations->fattr)
+               goto out_free_2;
 
        /* Get locations */
        dentry = ctx->clone_data.dentry;
@@ -427,14 +430,16 @@ static int nfs_do_refmount(struct fs_context *fc, struct rpc_clnt *client)
        err = nfs4_proc_fs_locations(client, d_inode(parent), &dentry->d_name, fs_locations, page);
        dput(parent);
        if (err != 0)
-               goto out_free_2;
+               goto out_free_3;
 
        err = -ENOENT;
        if (fs_locations->nlocations <= 0 ||
            fs_locations->fs_path.ncomponents <= 0)
-               goto out_free_2;
+               goto out_free_3;
 
        err = nfs_follow_referral(fc, fs_locations);
+out_free_3:
+       kfree(fs_locations->fattr);
 out_free_2:
        kfree(fs_locations);
 out_free:
index a79f66432bd3985cdfc486560ac5461ed6dd306f..0600f85b6016596f4a8b753f5d6f3e411a7c31db 100644 (file)
@@ -4243,6 +4243,8 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
        if (locations == NULL)
                goto out;
 
+       locations->fattr = fattr;
+
        status = nfs4_proc_fs_locations(client, dir, name, locations, page);
        if (status != 0)
                goto out;
@@ -4252,17 +4254,14 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
         * referral.  Cause us to drop into the exception handler, which
         * will kick off migration recovery.
         */
-       if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &locations->fattr.fsid)) {
+       if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &fattr->fsid)) {
                dprintk("%s: server did not return a different fsid for"
                        " a referral at %s\n", __func__, name->name);
                status = -NFS4ERR_MOVED;
                goto out;
        }
        /* Fixup attributes for the nfs_lookup() call to nfs_fhget() */
-       nfs_fixup_referral_attributes(&locations->fattr);
-
-       /* replace the lookup nfs_fattr with the locations nfs_fattr */
-       memcpy(fattr, &locations->fattr, sizeof(struct nfs_fattr));
+       nfs_fixup_referral_attributes(fattr);
        memset(fhandle, 0, sizeof(struct nfs_fh));
 out:
        if (page)
@@ -7902,7 +7901,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
        else
                bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;
 
-       nfs_fattr_init(&fs_locations->fattr);
+       nfs_fattr_init(fs_locations->fattr);
        fs_locations->server = server;
        fs_locations->nlocations = 0;
        status = nfs4_call_sync(client, server, &msg, &args.seq_args, &res.seq_res, 0);
@@ -7967,7 +7966,7 @@ static int _nfs40_proc_get_locations(struct nfs_server *server,
        unsigned long now = jiffies;
        int status;
 
-       nfs_fattr_init(&locations->fattr);
+       nfs_fattr_init(locations->fattr);
        locations->server = server;
        locations->nlocations = 0;
 
@@ -8032,7 +8031,7 @@ static int _nfs41_proc_get_locations(struct nfs_server *server,
        };
        int status;
 
-       nfs_fattr_init(&locations->fattr);
+       nfs_fattr_init(locations->fattr);
        locations->server = server;
        locations->nlocations = 0;
 
index 9e1c987c81e7f0901d3db7d4cb067dc61ed8a6fd..9656d40bb4887fc07c1fc18e9d7562fa7c76270c 100644 (file)
@@ -2106,6 +2106,11 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
                dprintk("<-- %s: no memory\n", __func__);
                goto out;
        }
+       locations->fattr = nfs_alloc_fattr();
+       if (locations->fattr == NULL) {
+               dprintk("<-- %s: no memory\n", __func__);
+               goto out;
+       }
 
        inode = d_inode(server->super->s_root);
        result = nfs4_proc_get_locations(server, NFS_FH(inode), locations,
@@ -2120,7 +2125,7 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
        if (!locations->nlocations)
                goto out;
 
-       if (!(locations->fattr.valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
+       if (!(locations->fattr->valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
                dprintk("<-- %s: No fs_locations data, migration skipped\n",
                        __func__);
                goto out;
@@ -2145,6 +2150,8 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
 out:
        if (page != NULL)
                __free_page(page);
+       if (locations != NULL)
+               kfree(locations->fattr);
        kfree(locations);
        if (result) {
                pr_err("NFS: migration recovery failed (server %s)\n",
index 86a5f6516928e19f44f115ab1ceac92e490ac8fa..5d822594336dcf2ce259ba8994f84fd067440729 100644 (file)
@@ -7051,7 +7051,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
        if (res->migration) {
                xdr_enter_page(xdr, PAGE_SIZE);
                status = decode_getfattr_generic(xdr,
-                                       &res->fs_locations->fattr,
+                                       res->fs_locations->fattr,
                                         NULL, res->fs_locations,
                                         res->fs_locations->server);
                if (status)
@@ -7064,7 +7064,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
                        goto out;
                xdr_enter_page(xdr, PAGE_SIZE);
                status = decode_getfattr_generic(xdr,
-                                       &res->fs_locations->fattr,
+                                       res->fs_locations->fattr,
                                         NULL, res->fs_locations,
                                         res->fs_locations->server);
        }
index 2863e5a69c6abdd208284c0238d135f297037b9f..20e97329fe4636beeb528dd8fb810bf9b57af9c9 100644 (file)
@@ -1212,7 +1212,7 @@ struct nfs4_fs_location {
 
 #define NFS4_FS_LOCATIONS_MAXENTRIES 10
 struct nfs4_fs_locations {
-       struct nfs_fattr fattr;
+       struct nfs_fattr *fattr;
        const struct nfs_server *server;
        struct nfs4_pathname fs_path;
        int nlocations;