Commit 836fb216 authored by J. Bruce Fields's avatar J. Bruce Fields Committed by Greg Kroah-Hartman

nfsd4: fix null dereference on replay

commit 9a307403 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: default avatarScott Mayhew <smayhew@redhat.com>
Signed-off-by: default avatarJ. Bruce Fields <bfields@redhat.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 8015cc81
...@@ -1769,6 +1769,12 @@ nfsd4_proc_compound(struct svc_rqst *rqstp, ...@@ -1769,6 +1769,12 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
opdesc->op_get_currentstateid(cstate, &op->u); opdesc->op_get_currentstateid(cstate, &op->u);
op->status = opdesc->op_func(rqstp, 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 (!op->status) {
if (opdesc->op_set_currentstateid) if (opdesc->op_set_currentstateid)
opdesc->op_set_currentstateid(cstate, &op->u); opdesc->op_set_currentstateid(cstate, &op->u);
...@@ -1779,14 +1785,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp, ...@@ -1779,14 +1785,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
if (need_wrongsec_check(rqstp)) if (need_wrongsec_check(rqstp))
op->status = check_nfsd_access(current_fh->fh_export, rqstp); op->status = check_nfsd_access(current_fh->fh_export, rqstp);
} }
encode_op: 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) { if (op->status == nfserr_replay_me) {
op->replay = &cstate->replay_owner->so_replay; op->replay = &cstate->replay_owner->so_replay;
nfsd4_encode_replay(&resp->xdr, op); nfsd4_encode_replay(&resp->xdr, op);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment