Patch "nfsd4: fix null dereference on replay" has been added to the 4.9-stable tree

This is a note to let you know that I've just added the patch titled

nfsd4: fix null dereference on replay

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
nfsd4-fix-null-dereference-on-replay.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: "J. Bruce Fields" <[email protected]>
Date: Tue, 23 May 2017 12:24:40 -0400
Subject: nfsd4: fix null dereference on replay

From: J. Bruce Fields <[email protected]>

commit 9a307403d374b993061f5992a6e260c944920d0b upstream.

if we receive a compound such that:

- the sessionid, slot, and sequence number in the SEQUENCE op match a cached succesful reply with N ops, and
- the Nth operation of the compound is a PUTFH, PUTPUBFH, PUTROOTFH, or RESTOREFH,

then nfsd4_sequence will return 0 and set cstate->status to nfserr_replay_cache. The current filehandle will not be set. This will cause us to call check_nfsd_access with first argument NULL.

To nfsd4_compound it looks like we just succesfully executed an operation that set a filehandle, but the current filehandle is not set.

Fix this by moving the nfserr_replay_cache earlier. There was never any reason to have it after the encode_op label, since the only case where he hit that is when opdesc->op_func sets it.

Note that there are two ways we could hit this case:

- a client is resending a previously sent compound that ended with one of the four PUTFH-like operations, or
- a client is sending a *new* compound that (incorrectly) shares sessionid, slot, and sequence number with a previously sent compound, and the length of the previously sent compound happens to match the position of a PUTFH-like operation in the new compound.

The second is obviously incorrect client behavior. The first is also very strange--the only purpose of a PUTFH-like operation is to set the current filehandle to be used by the following operation, so there's no point in having it as the last in a compound.

So it's likely this requires a buggy or malicious client to reproduce.

---
 fs/nfsd/nfs4proc.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

@@ -1783,6 +1783,12 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 			opdesc->op_get_currentstateid(cstate, &op->u);
 		op->status = opdesc->op_func(rqstp, cstate, &op->u);
 
+		/* Only from SEQUENCE */
+		if (cstate->status == nfserr_replay_cache) {
+			dprintk("%s NFS4.1 replay from cache\n", __func__);
+			status = op->status;
+			goto out;
+		}
 		if (!op->status) {
 			if (opdesc->op_set_currentstateid)
 				opdesc->op_set_currentstateid(cstate, &op->u);

@@ -1793,14 +1799,7 @@ nfsd4_proc_compound(struct svc_rqst *rqs
 			if (need_wrongsec_check(rqstp))
 				op->status = check_nfsd_access(current_fh->fh_export, rqstp);
 		}
-
 encode_op:
-		/* Only from SEQUENCE */
-		if (cstate->status == nfserr_replay_cache) {
-			dprintk("%s NFS4.1 replay from cache\n", __func__);
-			status = op->status;
-			goto out;
-		}
 		if (op->status == nfserr_replay_me) {
 			op->replay = &cstate->replay_owner->so_replay;
 			nfsd4_encode_replay(&resp->xdr, op);

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/nfsd4-fix-null-dereference-on-replay.patch queue-4.9/nfsd-fix-up-the-supattr_exclcreat-attributes.patch