nfsd: never defer requests during idmap lookup

During v4 request compound arg decoding, some ops (e.g. SETATTR)
can trigger idmap lookup upcalls. When those upcall responses get
delayed beyond the allowed time limit, cache_check() will mark the
request for deferral and cause it to be dropped.

This prevents nfs4svc_encode_compoundres from being executed, and
thus the session slot flag NFSD4_SLOT_INUSE never gets cleared.
Subsequent client requests will fail with NFSERR_JUKEBOX, given
that the slot will be marked as in-use, making the SEQUENCE op
fail.

Fix this by making sure that the RQ_USEDEFERRAL flag is always
clear during nfs4svc_decode_compoundargs(), since no v4 request
should ever be deferred.

Fixes: 2f425878b6a7 ("nfsd: don't use the deferral service, return NFS4ERR_DELAY")
Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
Reviewed-by: NeilBrown <neil@brown.name>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 8cca1329f3485c9f69783efda7dba0b752784dbd..b5b3d45979c9b3d58c7aec351fee321cba1b521f 100644 (file)
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -643,13 +643,31 @@ static __be32 encode_name_from_id(struct xdr_stream *xdr,
        return idmap_id_to_name(xdr, rqstp, type, id);
 }
 
-__be32
-nfsd_map_name_to_uid(struct svc_rqst *rqstp, const char *name, size_t namelen,
-               kuid_t *uid)
+/**
+ * nfsd_map_name_to_uid - Map user@domain to local UID
+ * @rqstp: RPC execution context
+ * @name: user@domain name to be mapped
+ * @namelen: length of name, in bytes
+ * @uid: OUT: mapped local UID value
+ *
+ * Returns nfs_ok on success or an NFSv4 status code on failure.
+ */
+__be32 nfsd_map_name_to_uid(struct svc_rqst *rqstp, const char *name,
+                           size_t namelen, kuid_t *uid)
 {
        __be32 status;
        u32 id = -1;
 
+       /*
+        * The idmap lookup below triggers an upcall that invokes
+        * cache_check(). RQ_USEDEFERRAL must be clear to prevent
+        * cache_check() from setting RQ_DROPME via svc_defer().
+        * NFSv4 servers are not permitted to drop requests. Also
+        * RQ_DROPME will force NFSv4.1 session slot processing to
+        * be skipped.
+        */
+       WARN_ON_ONCE(test_bit(RQ_USEDEFERRAL, &rqstp->rq_flags));
+
        if (name == NULL || namelen == 0)
                return nfserr_inval;
 
@@ -660,13 +678,31 @@ nfsd_map_name_to_uid(struct svc_rqst *rqstp, const char *name, size_t namelen,
        return status;
 }
 
-__be32
-nfsd_map_name_to_gid(struct svc_rqst *rqstp, const char *name, size_t namelen,
-               kgid_t *gid)
+/**
+ * nfsd_map_name_to_gid - Map user@domain to local GID
+ * @rqstp: RPC execution context
+ * @name: user@domain name to be mapped
+ * @namelen: length of name, in bytes
+ * @gid: OUT: mapped local GID value
+ *
+ * Returns nfs_ok on success or an NFSv4 status code on failure.
+ */
+__be32 nfsd_map_name_to_gid(struct svc_rqst *rqstp, const char *name,
+                           size_t namelen, kgid_t *gid)
 {
        __be32 status;
        u32 id = -1;
 
+       /*
+        * The idmap lookup below triggers an upcall that invokes
+        * cache_check(). RQ_USEDEFERRAL must be clear to prevent
+        * cache_check() from setting RQ_DROPME via svc_defer().
+        * NFSv4 servers are not permitted to drop requests. Also
+        * RQ_DROPME will force NFSv4.1 session slot processing to
+        * be skipped.
+        */
+       WARN_ON_ONCE(test_bit(RQ_USEDEFERRAL, &rqstp->rq_flags));
+
        if (name == NULL || namelen == 0)
                return nfserr_inval;
 

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4c708cf028490f1ff7281127684c48436a20192c..2b805fc51262d268c4d9ee94d81d1d8837cb0314 100644 (file)
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -3013,8 +3013,6 @@ encode_op:
        BUG_ON(cstate->replay_owner);
 out:
        cstate->status = status;
-       /* Reset deferral mechanism for RPC deferrals */
-       set_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
        return rpc_success;
 }
 

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 51ef97c2545688c7e0977281c810d79b21c63d54..5065727204b958c94f7b9a307ea4273b266b51c7 100644 (file)
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -6013,6 +6013,22 @@ nfs4svc_decode_compoundargs(struct svc_rqst *rqstp, struct xdr_stream *xdr)
        args->ops = args->iops;
        args->rqstp = rqstp;
 
+       /*
+        * NFSv4 operation decoders can invoke svc cache lookups
+        * that trigger svc_defer() when RQ_USEDEFERRAL is set,
+        * setting RQ_DROPME. This creates two problems:
+        *
+        * 1. Non-idempotency: Compounds make it too hard to avoid
+        *    problems if a request is deferred and replayed.
+        *
+        * 2. Session slot leakage (NFSv4.1+): If RQ_DROPME is set
+        *    during decode but SEQUENCE executes successfully, the
+        *    session slot will be marked INUSE. The request is then
+        *    dropped before encoding, so the slot is never released,
+        *    rendering it permanently unusable by the client.
+        */
+       clear_bit(RQ_USEDEFERRAL, &rqstp->rq_flags);
+
        return nfsd4_decode_compound(args);
 }