NFSD: Fix handling of oversized NFSv4 COMPOUND requests
authorChuck Lever <chuck.lever@oracle.com>
Mon, 5 Sep 2022 19:33:32 +0000 (15:33 -0400)
committerChuck Lever <chuck.lever@oracle.com>
Mon, 26 Sep 2022 18:02:30 +0000 (14:02 -0400)
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: 0078117c6d91 ("nfsd: return RESOURCE not GARBAGE_ARGS on too many ops")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
fs/nfsd/nfs4proc.c
fs/nfsd/nfs4xdr.c
fs/nfsd/xdr4.h

index 7055e1c91d0ef5d63e14a8c44c138708c96d7606..6ef0795fefb450d3a2c60b089220f66bd5d7c509 100644 (file)
@@ -2631,9 +2631,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) {
@@ -2646,10 +2643,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
@@ -2725,8 +2732,8 @@ encode_op:
                        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);
index 4b69e86240eb2fae59aacb95190d458cf32c34dc..09519b4a860a975a6255127cec1491f3c3482047 100644 (file)
@@ -2359,16 +2359,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 = vcalloc(argp->opcnt, sizeof(*argp->ops));
index 96267258e629100943fe980a1f97e056623ba841..466e2786fc976ca3d1fcb9c460dc172366475794 100644 (file)
@@ -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];