NFSv4: Place the GETATTR operation before the CLOSE
authorTrond Myklebust <trond.myklebust@primarydata.com>
Mon, 19 Dec 2016 17:14:44 +0000 (12:14 -0500)
committerTrond Myklebust <trond.myklebust@primarydata.com>
Mon, 19 Dec 2016 22:30:00 +0000 (17:30 -0500)
In order to benefit from the DENY share lock protection, we should
put the GETATTR operation before the CLOSE. Otherwise, we might race
with a Windows machine that thinks it is now safe to modify the file.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
fs/nfs/nfs4proc.c
fs/nfs/nfs4xdr.c

index 872ff6756723b9e1a54b7e55737cc276542b8197..c3263114c6b8ef11b39978da1953cbffa357fcc8 100644 (file)
@@ -3147,7 +3147,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
                        res_stateid, calldata->arg.fmode);
 out_release:
        nfs_release_seqid(calldata->arg.seqid);
-       nfs_refresh_inode(calldata->inode, calldata->res.fattr);
+       nfs_refresh_inode(calldata->inode, &calldata->fattr);
        dprintk("%s: done, ret = %d!\n", __func__, task->tk_status);
 }
 
@@ -3215,7 +3215,10 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
                nfs4_map_atomic_open_share(NFS_SERVER(inode),
                                calldata->arg.fmode, 0);
 
-       nfs_fattr_init(calldata->res.fattr);
+       if (calldata->res.fattr == NULL)
+               calldata->arg.bitmask = NULL;
+       else if (calldata->arg.bitmask == NULL)
+               calldata->res.fattr = NULL;
        calldata->timestamp = jiffies;
        if (nfs4_setup_sequence(NFS_SERVER(inode),
                                &calldata->arg.seq_args,
@@ -3282,6 +3285,7 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait)
        calldata->arg.seqid = alloc_seqid(&state->owner->so_seqid, gfp_mask);
        if (IS_ERR(calldata->arg.seqid))
                goto out_free_calldata;
+       nfs_fattr_init(&calldata->fattr);
        calldata->arg.fmode = 0;
        calldata->lr.arg.ld_private = &calldata->lr.ld_private;
        calldata->res.fattr = &calldata->fattr;
index 8ccb34cf0c193dcc5c8fa8c8e77b759450732d0e..6f2365b99fb441959ac856e10a01ba2414abcdcd 100644 (file)
@@ -2279,9 +2279,9 @@ static void nfs4_xdr_enc_close(struct rpc_rqst *req, struct xdr_stream *xdr,
        encode_putfh(xdr, args->fh, &hdr);
        if (args->lr_args)
                encode_layoutreturn(xdr, args->lr_args, &hdr);
-       encode_close(xdr, args, &hdr);
        if (args->bitmask != NULL)
                encode_getfattr(xdr, args->bitmask, &hdr);
+       encode_close(xdr, args, &hdr);
        encode_nops(&hdr);
 }
 
@@ -6494,16 +6494,12 @@ static int nfs4_xdr_dec_close(struct rpc_rqst *rqstp, struct xdr_stream *xdr,
                if (status)
                        goto out;
        }
+       if (res->fattr != NULL) {
+               status = decode_getfattr(xdr, res->fattr, res->server);
+               if (status != 0)
+                       goto out;
+       }
        status = decode_close(xdr, res);
-       if (status != 0)
-               goto out;
-       /*
-        * Note: Server may do delete on close for this file
-        *      in which case the getattr call will fail with
-        *      an ESTALE error. Shouldn't be a problem,
-        *      though, since fattr->valid will remain unset.
-        */
-       decode_getfattr(xdr, res->fattr, res->server);
 out:
        return status;
 }