Skip to content

Commit

Permalink
NFSD: Fix handling of oversized NFSv4 COMPOUND requests
Browse files Browse the repository at this point in the history
[ Upstream commit 7518a3d ]

If an NFS server returns NFS4ERR_RESOURCE on the first operation in
an NFSv4 COMPOUND, there's no way for a client to know where the
problem is and then simplify the compound to make forward progress.

So instead, make NFSD process as many operations in an oversized
COMPOUND as it can and then return NFS4ERR_RESOURCE on the first
operation it did not process.

pynfs NFSv4.0 COMP6 exercises this case, but checks only for the
COMPOUND status code, not whether the server has processed any
of the operations.

pynfs NFSv4.1 SEQ6 and SEQ7 exercise the NFSv4.1 case, which detects
too many operations per COMPOUND by checking against the limits
negotiated when the session was created.

Suggested-by: Bruce Fields <bfields@fieldses.org>
Fixes: 0078117 ("nfsd: return RESOURCE not GARBAGE_ARGS on too many ops")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
chucklever authored and gregkh committed Oct 21, 2022
1 parent f59c74d commit 46841a9
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 16 deletions.
19 changes: 13 additions & 6 deletions fs/nfsd/nfs4proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -2633,9 +2633,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
status = nfserr_minor_vers_mismatch;
if (nfsd_minorversion(nn, args->minorversion, NFSD_TEST) <= 0)
goto out;
status = nfserr_resource;
if (args->opcnt > NFSD_MAX_OPS_PER_COMPOUND)
goto out;

status = nfs41_check_op_ordering(args);
if (status) {
Expand All @@ -2648,10 +2645,20 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)

rqstp->rq_lease_breaker = (void **)&cstate->clp;

trace_nfsd_compound(rqstp, args->opcnt);
trace_nfsd_compound(rqstp, args->client_opcnt);
while (!status && resp->opcnt < args->opcnt) {
op = &args->ops[resp->opcnt++];

if (unlikely(resp->opcnt == NFSD_MAX_OPS_PER_COMPOUND)) {
/* If there are still more operations to process,
* stop here and report NFS4ERR_RESOURCE. */
if (cstate->minorversion == 0 &&
args->client_opcnt > resp->opcnt) {
op->status = nfserr_resource;
goto encode_op;
}
}

/*
* The XDR decode routines may have pre-set op->status;
* for example, if there is a miscellaneous XDR error
Expand Down Expand Up @@ -2727,8 +2734,8 @@ nfsd4_proc_compound(struct svc_rqst *rqstp)
status = op->status;
}

trace_nfsd_compound_status(args->opcnt, resp->opcnt, status,
nfsd4_op_name(op->opnum));
trace_nfsd_compound_status(args->client_opcnt, resp->opcnt,
status, nfsd4_op_name(op->opnum));

nfsd4_cstate_clear_replay(cstate);
nfsd4_increment_op_stats(op->opnum);
Expand Down
12 changes: 3 additions & 9 deletions fs/nfsd/nfs4xdr.c
Original file line number Diff line number Diff line change
Expand Up @@ -2357,16 +2357,10 @@ nfsd4_decode_compound(struct nfsd4_compoundargs *argp)

if (xdr_stream_decode_u32(argp->xdr, &argp->minorversion) < 0)
return false;
if (xdr_stream_decode_u32(argp->xdr, &argp->opcnt) < 0)
if (xdr_stream_decode_u32(argp->xdr, &argp->client_opcnt) < 0)
return false;

/*
* NFS4ERR_RESOURCE is a more helpful error than GARBAGE_ARGS
* here, so we return success at the xdr level so that
* nfsd4_proc can handle this is an NFS-level error.
*/
if (argp->opcnt > NFSD_MAX_OPS_PER_COMPOUND)
return true;
argp->opcnt = min_t(u32, argp->client_opcnt,
NFSD_MAX_OPS_PER_COMPOUND);

if (argp->opcnt > ARRAY_SIZE(argp->iops)) {
argp->ops = kzalloc(argp->opcnt * sizeof(*argp->ops), GFP_KERNEL);
Expand Down
3 changes: 2 additions & 1 deletion fs/nfsd/xdr4.h
Original file line number Diff line number Diff line change
Expand Up @@ -717,9 +717,10 @@ struct nfsd4_compoundargs {
struct svcxdr_tmpbuf *to_free;
struct svc_rqst *rqstp;

u32 taglen;
char * tag;
u32 taglen;
u32 minorversion;
u32 client_opcnt;
u32 opcnt;
struct nfsd4_op *ops;
struct nfsd4_op iops[8];
Expand Down

0 comments on commit 46841a9

Please sign in to comment.