]> git.proxmox.com Git - mirror_ubuntu-zesty-kernel.git/commitdiff
nfsd4: fix null dereference on replay
authorJ. Bruce Fields <bfields@redhat.com>
Tue, 23 May 2017 16:24:40 +0000 (12:24 -0400)
committerThadeu Lima de Souza Cascardo <cascardo@canonical.com>
Tue, 27 Jun 2017 13:16:24 +0000 (10:16 -0300)
BugLink: http://bugs.launchpad.net/bugs/1698799
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.

Reported-by: Scott Mayhew <smayhew@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
fs/nfsd/nfs4proc.c

index 7d5351cd67fb09ff5aeb775f2e71f119f51f1800..209dbfc50cd45853c42d783d2e5dc3ed0454234a 100644 (file)
@@ -1690,6 +1690,12 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
                        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);
@@ -1700,14 +1706,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
                        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);