[PATCH] nfsd: prevent a double put of revoked layout stateids

[PATCH] nfsd: prevent a double put of revoked layout stateids

[Jinpyo Lee]

nfsd4_free_stateid() finds an admin-revoked layout stateid with
find_stateid_locked(), which does not acquire a reference. Since the
layout case does not mark the stateid closed before dropping cl_lock,
two FREE_STATEID operations in separate session slots can both select
the same stateid and drop a reference they do not own.

With fence-work teardown in place, the second caller can also wait in
nfsd4_stop_layout_fence() until the first caller frees the stateid, then
resume through its freed fence mutex and layout lock.

Mark the layout stateid CLOSED under cl_lock and ls_lock so only one
disposal caller proceeds. Take a temporary reference before dropping
cl_lock and hold it while stopping fence work and draining the remaining
layout segments, then release that reference with the final put.

The layout stateid IDR does not own a separate reference. The initial
allocation reference is released when LAYOUTGET completes, and each
layout segment holds its own reference. Serialize segment draining with
in-flight layout operations using ls_mutex, and recheck CLOSED after
acquiring that mutex so a stale LAYOUTGET cannot add a segment after the
drain. Also make the newly allocated layout stateid the error-path put
target.

Read CLOSED under ls_lock in FREE_STATEID, and restrict
nfs40_clean_admin_revoked() to NFSv4.0 clients. Session clients dispose
of revoked stateids through FREE_STATEID and must not be processed by
the NFSv4.0 cleanup while layout disposal is in progress.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
Assisted-by: LLM
Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
---
Validated on nfsd-testing d918c664efd5ec6a7309e98f567fb018bcde0e11
(2026-10-12):
- Applied cleanly and passed strict checkpatch with no errors or warnings.
- Built and booted an x86_64 kernel and modules with GCC 13.3, W=1,
  Generic KASAN, lockdep, and pNFS block and SCSI layouts enabled.
- NFSv4.2, NFSv4.0, and NFSv3 read/write, fio, and unmount tests passed.
- On a real loopback pNFS SCSI layout, administrator revocation followed
  by one FREE_STATEID removed the layout stateid and reduced the
  admin-revoked count from 2 to 1. The sequence passed twice.
- Tracing observed nfsd4_stop_layout_fence() in both revocation and
  FREE_STATEID processing, followed by nfsd_layoutstate_free.
- No KASAN, lockdep, refcount, list-corruption, or Oops report occurred;
  kernel taint remained zero.

The full xfstests suite and a forced concurrent two-slot FREE_STATEID
race were not run on this revision.

 fs/nfsd/nfs4layouts.c | 24 ++++++++++++++++++++++++
 fs/nfsd/nfs4state.c   | 37 ++++++++++++++++++++++++++++++++++++-
 fs/nfsd/pnfs.h        |  6 ++++++
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 898776c9d..74c39e216 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 		status = nfserr_jukebox;
 		if (!ls)
 			goto out;
+		stid = &ls->ls_stid;
 		mutex_lock(&ls->ls_mutex);
 	} else {
 		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
@@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 			goto out_unlock_stid;
 	}
 
+	/* A new or looked-up stateid may have been closed before ls_mutex. */
+	spin_lock(&ls->ls_lock);
+	if (ls->ls_stid.sc_status & SC_STATUS_CLOSED) {
+		spin_unlock(&ls->ls_lock);
+		status = nfserr_bad_stateid;
+		goto out_unlock_stid;
+	}
+	spin_unlock(&ls->ls_lock);
+
 	*lsp = ls;
 	return 0;
 
@@ -594,6 +604,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
 	spin_unlock(&ls->ls_lock);
 }
 
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+	LIST_HEAD(reaplist);
+
+	/*
+	 * The caller holds a stateid reference until segment draining ends.
+	 * Wait for any LAYOUTGET that passed the CLOSED check.
+	 */
+	mutex_lock(&ls->ls_mutex);
+	nfsd4_return_all_layouts(ls, &reaplist);
+	mutex_unlock(&ls->ls_mutex);
+	nfsd4_free_layouts(&reaplist);
+}
+
 void
 nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 920967dd6..02bfbd242 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5837,6 +5837,24 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
 	return ret;
 }
 
+/* cl_lock is held; layout stateid status also requires ls_lock. */
+static bool nfsd4_stid_is_closed(struct nfs4_stid *s)
+{
+	struct nfs4_layout_stateid *ls;
+	bool closed;
+
+	lockdep_assert_held(&s->sc_client->cl_lock);
+
+	if (s->sc_type != SC_TYPE_LAYOUT)
+		return s->sc_status & SC_STATUS_CLOSED;
+
+	ls = layoutstateid(s);
+	spin_lock(&ls->ls_lock);
+	closed = s->sc_status & SC_STATUS_CLOSED;
+	spin_unlock(&ls->ls_lock);
+	return closed;
+}
+
 static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 	__releases(&s->sc_client->cl_lock)
 {
@@ -5871,9 +5889,24 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 		break;
 	case SC_TYPE_LAYOUT:
 		ls = layoutstateid(s);
+		spin_lock(&ls->ls_lock);
+		if (s->sc_status & SC_STATUS_CLOSED) {
+			spin_unlock(&ls->ls_lock);
+			spin_unlock(&cl->cl_lock);
+			break;
+		}
+		s->sc_status |= SC_STATUS_CLOSED;
+		spin_unlock(&ls->ls_lock);
+		/*
+		 * The IDR does not own a layout stateid reference. Keep ls
+		 * alive while draining worker and segment references below;
+		 * the final put releases this temporary reference.
+		 */
+		refcount_inc(&s->sc_count);
 		list_del_init(&ls->ls_perclnt);
 		spin_unlock(&cl->cl_lock);
 		nfsd4_stop_layout_fence(ls);
+		nfsd4_return_revoked_layout(ls);
 		nfs4_put_stid(s);
 		break;
 	default:
@@ -7899,6 +7932,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
 		unsigned long id, tmp;
 		struct nfs4_stid *stid;
 
+		if (clp->cl_minorversion != 0)
+			continue;
 		if (atomic_read(&clp->cl_admin_revoked) == 0)
 			continue;
 		if (is_client_expired(clp))
@@ -8698,7 +8733,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	spin_lock(&cl->cl_lock);
 	s = find_stateid_locked(cl, stateid);
-	if (!s || s->sc_status & SC_STATUS_CLOSED)
+	if (!s || nfsd4_stid_is_closed(s))
 		goto out_unlock;
 	if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
 		nfsd4_drop_revoked_stid(s);
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index 559df0e5b..9b0f17a4d 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
 #ifdef CONFIG_NFSD_PNFS
 void nfsd4_setup_layout_type(struct svc_export *exp);
 void nfsd4_return_all_client_layouts(struct nfs4_client *);
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
 void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp);
 void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
@@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
 static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
 }
+
+static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+}
+
 static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp)
 {

base-commit: d918c664efd5ec6a7309e98f567fb018bcde0e11
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] nfsd: prevent a double put of revoked layout stateids

[Jeff Layton]

On Mon, 2026-10-12 at 17:47 +0900, Jinpyo Lee wrote:
> nfsd4_free_stateid() finds an admin-revoked layout stateid with
> find_stateid_locked(), which does not acquire a reference. Since the
> layout case does not mark the stateid closed before dropping cl_lock,
> two FREE_STATEID operations in separate session slots can both select
> the same stateid and drop a reference they do not own.
> 
> With fence-work teardown in place, the second caller can also wait in
> nfsd4_stop_layout_fence() until the first caller frees the stateid, then
> resume through its freed fence mutex and layout lock.
> 
> Mark the layout stateid CLOSED under cl_lock and ls_lock so only one
> disposal caller proceeds. Take a temporary reference before dropping
> cl_lock and hold it while stopping fence work and draining the remaining
> layout segments, then release that reference with the final put.
> 
> The layout stateid IDR does not own a separate reference. The initial
> allocation reference is released when LAYOUTGET completes, and each
> layout segment holds its own reference. Serialize segment draining with
> in-flight layout operations using ls_mutex, and recheck CLOSED after
> acquiring that mutex so a stale LAYOUTGET cannot add a segment after the
> drain. Also make the newly allocated layout stateid the error-path put
> target.
> 
> Read CLOSED under ls_lock in FREE_STATEID, and restrict
> nfs40_clean_admin_revoked() to NFSv4.0 clients. Session clients dispose
> of revoked stateids through FREE_STATEID and must not be processed by
> the NFSv4.0 cleanup while layout disposal is in progress.
> 
> The vulnerability research and validation were conducted by members of
> the Tobabz team as part of the Best of the Best 15th program.
> 
> Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
> Assisted-by: LLM
> Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
> ---
> Validated on nfsd-testing d918c664efd5ec6a7309e98f567fb018bcde0e11
> (2026-10-12):
> - Applied cleanly and passed strict checkpatch with no errors or warnings.
> - Built and booted an x86_64 kernel and modules with GCC 13.3, W=1,
>   Generic KASAN, lockdep, and pNFS block and SCSI layouts enabled.
> - NFSv4.2, NFSv4.0, and NFSv3 read/write, fio, and unmount tests passed.
> - On a real loopback pNFS SCSI layout, administrator revocation followed
>   by one FREE_STATEID removed the layout stateid and reduced the
>   admin-revoked count from 2 to 1. The sequence passed twice.
> - Tracing observed nfsd4_stop_layout_fence() in both revocation and
>   FREE_STATEID processing, followed by nfsd_layoutstate_free.
> - No KASAN, lockdep, refcount, list-corruption, or Oops report occurred;
>   kernel taint remained zero.
> 
> The full xfstests suite and a forced concurrent two-slot FREE_STATEID
> race were not run on this revision.
> 
>  fs/nfsd/nfs4layouts.c | 24 ++++++++++++++++++++++++
>  fs/nfsd/nfs4state.c   | 37 ++++++++++++++++++++++++++++++++++++-
>  fs/nfsd/pnfs.h        |  6 ++++++
>  3 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 898776c9d..74c39e216 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  		status = nfserr_jukebox;
>  		if (!ls)
>  			goto out;
> +		stid = &ls->ls_stid;
>  		mutex_lock(&ls->ls_mutex);
>  	} else {
>  		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
> @@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  			goto out_unlock_stid;
>  	}
>  
> +	/* A new or looked-up stateid may have been closed before ls_mutex. */
> +	spin_lock(&ls->ls_lock);
> +	if (ls->ls_stid.sc_status & SC_STATUS_CLOSED) {
> +		spin_unlock(&ls->ls_lock);
> +		status = nfserr_bad_stateid;
> +		goto out_unlock_stid;
> +	}
> +	spin_unlock(&ls->ls_lock);
> +
>  	*lsp = ls;
>  	return 0;
>  
> @@ -594,6 +604,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
>  	spin_unlock(&ls->ls_lock);
>  }
>  
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +	LIST_HEAD(reaplist);
> +
> +	/*
> +	 * The caller holds a stateid reference until segment draining ends.
> +	 * Wait for any LAYOUTGET that passed the CLOSED check.
> +	 */
> +	mutex_lock(&ls->ls_mutex);
> +	nfsd4_return_all_layouts(ls, &reaplist);
> +	mutex_unlock(&ls->ls_mutex);
> +	nfsd4_free_layouts(&reaplist);
> +}
> +
>  void
>  nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 920967dd6..02bfbd242 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5837,6 +5837,24 @@ nfsd4_find_existing_open(struct nfs4_file *fp, struct nfsd4_open *open)
>  	return ret;
>  }
>  
> +/* cl_lock is held; layout stateid status also requires ls_lock. */
> +static bool nfsd4_stid_is_closed(struct nfs4_stid *s)
> +{
> +	struct nfs4_layout_stateid *ls;
> +	bool closed;
> +
> +	lockdep_assert_held(&s->sc_client->cl_lock);
> +
> +	if (s->sc_type != SC_TYPE_LAYOUT)
> +		return s->sc_status & SC_STATUS_CLOSED;
> +
> +	ls = layoutstateid(s);
> +	spin_lock(&ls->ls_lock);
> +	closed = s->sc_status & SC_STATUS_CLOSED;
> +	spin_unlock(&ls->ls_lock);
> +	return closed;
> +}
> +

^^^ 
I must say that I'm not thrilled with the different locking rules for
SC_STATUS_CLOSED with different types of stateids. That has a strong
code smell in my opinion and will be difficult (for humans) to keep
straight.

I don't have a better suggestion for the moment, but I'd strongly
prefer something that didn't add these extra rules around a flag common
to all stateids.

>  static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  	__releases(&s->sc_client->cl_lock)
>  {
> @@ -5871,9 +5889,24 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  		break;
>  	case SC_TYPE_LAYOUT:
>  		ls = layoutstateid(s);
> +		spin_lock(&ls->ls_lock);
> +		if (s->sc_status & SC_STATUS_CLOSED) {
> +			spin_unlock(&ls->ls_lock);
> +			spin_unlock(&cl->cl_lock);
> +			break;
> +		}
> +		s->sc_status |= SC_STATUS_CLOSED;
> +		spin_unlock(&ls->ls_lock);
> +		/*
> +		 * The IDR does not own a layout stateid reference. Keep ls
> +		 * alive while draining worker and segment references below;
> +		 * the final put releases this temporary reference.
> +		 */
> +		refcount_inc(&s->sc_count);
>  		list_del_init(&ls->ls_perclnt);
>  		spin_unlock(&cl->cl_lock);
>  		nfsd4_stop_layout_fence(ls);
> +		nfsd4_return_revoked_layout(ls);
>  		nfs4_put_stid(s);
>  		break;
>  	default:
> @@ -7899,6 +7932,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
>  		unsigned long id, tmp;
>  		struct nfs4_stid *stid;
>  
> +		if (clp->cl_minorversion != 0)
> +			continue;
>  		if (atomic_read(&clp->cl_admin_revoked) == 0)
>  			continue;
>  		if (is_client_expired(clp))
> @@ -8698,7 +8733,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	spin_lock(&cl->cl_lock);
>  	s = find_stateid_locked(cl, stateid);
> -	if (!s || s->sc_status & SC_STATUS_CLOSED)
> +	if (!s || nfsd4_stid_is_closed(s))
>  		goto out_unlock;
>  	if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
>  		nfsd4_drop_revoked_stid(s);
> diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
> index 559df0e5b..9b0f17a4d 100644
> --- a/fs/nfsd/pnfs.h
> +++ b/fs/nfsd/pnfs.h
> @@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
>  #ifdef CONFIG_NFSD_PNFS
>  void nfsd4_setup_layout_type(struct svc_export *exp);
>  void nfsd4_return_all_client_layouts(struct nfs4_client *);
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
>  void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp);
>  void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
> @@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
>  static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
>  }
> +
> +static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +}
> +
>  static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp)
>  {
> 
> base-commit: d918c664efd5ec6a7309e98f567fb018bcde0e11

-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v2] nfsd: prevent a double put of revoked layout stateids

[Jinpyo Lee]

nfsd4_free_stateid() finds an admin-revoked layout stateid with
find_stateid_locked(), which does not acquire a reference. The layout
case drops cl_lock without marking the stateid closed, then stops fence
work and calls nfs4_put_stid() without owning a reference. Concurrent
disposal can therefore access a freed layout stateid or drop a reference
owned by another holder.

Mark the layout stateid CLOSED under cl_lock so only one disposal caller
proceeds. Take a temporary reference before dropping cl_lock and hold it
while stopping fence work and draining the remaining layout segments,
then release that reference with the final put. This keeps the stateid
alive even though its IDR owns no reference and its layout segments are
being removed.

Use cl_lock for the CLOSED update in LAYOUTRETURN as well, matching the
common stateid locking rule. Retain ls_mutex from the last segment removal
through the CLOSED update, and recheck CLOSED under cl_lock after taking
ls_mutex in LAYOUTGET. This prevents a stale LAYOUTGET from adding a
segment after the drain. Also make the newly allocated layout stateid the
error-path put target and update the status-locking comment.

Restrict nfs40_clean_admin_revoked() to NFSv4.0 clients. Its timer is
armed only by v4.0 revocations, while session clients dispose of revoked
stateids through FREE_STATEID. Skipping session clients prevents the scan
from repeatedly selecting a CLOSED stateid that remains in the IDR while
an in-flight operation holds a reference.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
Suggested-by: Chuck Lever <cel@kernel.org>
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
---
Changes since v1:
- Address Jeff Layton's concern by using cl_lock for layout CLOSED
  checks and updates, without a type-dependent stateid status helper.
- Move LAYOUTRETURN's CLOSED update outside ls_lock while retaining
  ls_mutex, and update the status-locking documentation.
- Clarify the LAYOUTGET serialization and v4.0 cleanup restriction.
- Credit Chuck Lever's suggestion and mark the fix for stable review.
- Rebase onto nfsd-testing c807415bc69013a5f162264ee0e14f8742c182f7.

Validated on the above nfsd-testing base (2026-10-13):
- Applied cleanly, passed strict checkpatch, and built and booted an x86_64
  kernel with GCC 13.3, W=1, Generic KASAN, lockdep, and pNFS block and
  SCSI layouts enabled.
- Loopback pNFS SCSI read, shrink, integrity, revocation, and cleanup tests
  passed, as did NFSv4.2, NFSv4.0, and NFSv3 read/write, fio CRC32C, and
  unmount tests. Tracing observed two successful LAYOUTRETURNs reach the
  modified CLOSED update and release their stateids.
- Two concurrent FREE_STATEID requests from different slots of one NFSv4.1
  session returned one success and one NFS4ERR_BAD_STATEID. A test-only
  two-second pause forced the losing request into the disposal window;
  tracing observed CLOSED, exactly one revoked-layout drain, and one
  layout-state free.
- No KASAN, lockdep, refcount, list-corruption, WARNING, or Oops report
  occurred, and kernel taint remained zero through fixture cleanup.

Build note: Existing format-truncation warnings in target_core_transport.c
were excluded from WERROR; W=1 and WERROR remained enabled otherwise.

The full xfstests suite was not run. Earlier v1 runtime results are not
used as validation of this revision.

 fs/nfsd/nfs4layouts.c | 34 ++++++++++++++++++++++++++++++++--
 fs/nfsd/nfs4state.c   | 14 ++++++++++++++
 fs/nfsd/pnfs.h        |  6 ++++++
 fs/nfsd/state.h       |  5 ++---
 4 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 898776c9d..749ace97e 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 		status = nfserr_jukebox;
 		if (!ls)
 			goto out;
+		stid = &ls->ls_stid;
 		mutex_lock(&ls->ls_mutex);
 	} else {
 		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
@@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 			goto out_unlock_stid;
 	}
 
+	/* A new or looked-up stateid may have been closed before ls_mutex. */
+	spin_lock(&stid->sc_client->cl_lock);
+	if (stid->sc_status & SC_STATUS_CLOSED) {
+		spin_unlock(&stid->sc_client->cl_lock);
+		status = nfserr_bad_stateid;
+		goto out_unlock_stid;
+	}
+	spin_unlock(&stid->sc_client->cl_lock);
+
 	*lsp = ls;
 	return 0;
 
@@ -537,12 +547,18 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
 			nfs4_inc_and_copy_stateid(&lrp->lr_sid, &ls->ls_stid);
 		lrp->lrs_present = true;
 	} else {
-		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
-		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
 		lrp->lrs_present = false;
 	}
 	spin_unlock(&ls->ls_lock);
 
+	if (!lrp->lrs_present) {
+		/* ls_mutex prevents a LAYOUTGET from adding another segment. */
+		spin_lock(&ls->ls_stid.sc_client->cl_lock);
+		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
+		spin_unlock(&ls->ls_stid.sc_client->cl_lock);
+		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
+	}
+
 	mutex_unlock(&ls->ls_mutex);
 	nfs4_put_stid(&ls->ls_stid);
 	nfsd4_free_layouts(&reaplist);
@@ -594,6 +610,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
 	spin_unlock(&ls->ls_lock);
 }
 
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+	LIST_HEAD(reaplist);
+
+	/*
+	 * The caller holds a stateid reference until segment draining ends.
+	 * Wait for any LAYOUTGET that passed the CLOSED check.
+	 */
+	mutex_lock(&ls->ls_mutex);
+	nfsd4_return_all_layouts(ls, &reaplist);
+	mutex_unlock(&ls->ls_mutex);
+	nfsd4_free_layouts(&reaplist);
+}
+
 void
 nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 26d2d56a5..1740b86eb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5873,9 +5873,21 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 		break;
 	case SC_TYPE_LAYOUT:
 		ls = layoutstateid(s);
+		if (s->sc_status & SC_STATUS_CLOSED) {
+			spin_unlock(&cl->cl_lock);
+			break;
+		}
+		s->sc_status |= SC_STATUS_CLOSED;
+		/*
+		 * The IDR does not own a layout stateid reference. Keep ls
+		 * alive while draining worker and segment references below;
+		 * the final put releases this temporary reference.
+		 */
+		refcount_inc(&s->sc_count);
 		list_del_init(&ls->ls_perclnt);
 		spin_unlock(&cl->cl_lock);
 		nfsd4_stop_layout_fence(ls);
+		nfsd4_return_revoked_layout(ls);
 		nfs4_put_stid(s);
 		break;
 	default:
@@ -7901,6 +7913,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
 		unsigned long id, tmp;
 		struct nfs4_stid *stid;
 
+		if (clp->cl_minorversion != 0)
+			continue;
 		if (atomic_read(&clp->cl_admin_revoked) == 0)
 			continue;
 		if (is_client_expired(clp))
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index 559df0e5b..9b0f17a4d 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
 #ifdef CONFIG_NFSD_PNFS
 void nfsd4_setup_layout_type(struct svc_export *exp);
 void nfsd4_return_all_client_layouts(struct nfs4_client *);
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
 void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp);
 void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
@@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
 static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
 }
+
+static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+}
+
 static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp)
 {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 79dfa47b8..63b581104 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -148,9 +148,8 @@ struct nfs4_stid {
  * nn->deleg_lock protects sc_status for hashed delegation stateids.
  * ->cl_lock protects the bits set as one is disposed of
  * (SC_STATUS_CLOSED, SC_STATUS_FREEABLE, SC_STATUS_FREED) and
- * sc_status for open and lock stateids. ->st_mutex also protects
- * sc_status for open stateids. ->ls_lock protects sc_status for
- * layout stateids.
+ * sc_status for open, lock, and layout stateids. ->st_mutex also protects
+ * sc_status for open stateids.
  */
 /*
  * For an open stateid kept around *only* to process close replays.

base-commit: c807415bc69013a5f162264ee0e14f8742c182f7
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2] nfsd: prevent a double put of revoked layout stateids

[Jeff Layton]

On Tue, 2026-10-13 at 15:00 +0900, Jinpyo Lee wrote:
> nfsd4_free_stateid() finds an admin-revoked layout stateid with
> find_stateid_locked(), which does not acquire a reference. The layout
> case drops cl_lock without marking the stateid closed, then stops fence
> work and calls nfs4_put_stid() without owning a reference. Concurrent
> disposal can therefore access a freed layout stateid or drop a reference
> owned by another holder.
> 
> Mark the layout stateid CLOSED under cl_lock so only one disposal caller
> proceeds. Take a temporary reference before dropping cl_lock and hold it
> while stopping fence work and draining the remaining layout segments,
> then release that reference with the final put. This keeps the stateid
> alive even though its IDR owns no reference and its layout segments are
> being removed.
> 
> Use cl_lock for the CLOSED update in LAYOUTRETURN as well, matching the
> common stateid locking rule. Retain ls_mutex from the last segment removal
> through the CLOSED update, and recheck CLOSED under cl_lock after taking
> ls_mutex in LAYOUTGET. This prevents a stale LAYOUTGET from adding a
> segment after the drain. Also make the newly allocated layout stateid the
> error-path put target and update the status-locking comment.
> 
> Restrict nfs40_clean_admin_revoked() to NFSv4.0 clients. Its timer is
> armed only by v4.0 revocations, while session clients dispose of revoked
> stateids through FREE_STATEID. Skipping session clients prevents the scan
> from repeatedly selecting a CLOSED stateid that remains in the IDR while
> an in-flight operation holds a reference.
> 
> The vulnerability research and validation were conducted by members of
> the Tobabz team as part of the Best of the Best 15th program.
> 
> Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
> Suggested-by: Chuck Lever <cel@kernel.org>
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
> ---
> Changes since v1:
> - Address Jeff Layton's concern by using cl_lock for layout CLOSED
>   checks and updates, without a type-dependent stateid status helper.
> - Move LAYOUTRETURN's CLOSED update outside ls_lock while retaining
>   ls_mutex, and update the status-locking documentation.
> - Clarify the LAYOUTGET serialization and v4.0 cleanup restriction.
> - Credit Chuck Lever's suggestion and mark the fix for stable review.
> - Rebase onto nfsd-testing c807415bc69013a5f162264ee0e14f8742c182f7.
> 
> Validated on the above nfsd-testing base (2026-10-13):
> - Applied cleanly, passed strict checkpatch, and built and booted an x86_64
>   kernel with GCC 13.3, W=1, Generic KASAN, lockdep, and pNFS block and
>   SCSI layouts enabled.
> - Loopback pNFS SCSI read, shrink, integrity, revocation, and cleanup tests
>   passed, as did NFSv4.2, NFSv4.0, and NFSv3 read/write, fio CRC32C, and
>   unmount tests. Tracing observed two successful LAYOUTRETURNs reach the
>   modified CLOSED update and release their stateids.
> - Two concurrent FREE_STATEID requests from different slots of one NFSv4.1
>   session returned one success and one NFS4ERR_BAD_STATEID. A test-only
>   two-second pause forced the losing request into the disposal window;
>   tracing observed CLOSED, exactly one revoked-layout drain, and one
>   layout-state free.
> - No KASAN, lockdep, refcount, list-corruption, WARNING, or Oops report
>   occurred, and kernel taint remained zero through fixture cleanup.
> 
> Build note: Existing format-truncation warnings in target_core_transport.c
> were excluded from WERROR; W=1 and WERROR remained enabled otherwise.
> 
> The full xfstests suite was not run. Earlier v1 runtime results are not
> used as validation of this revision.
> 
>  fs/nfsd/nfs4layouts.c | 34 ++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfs4state.c   | 14 ++++++++++++++
>  fs/nfsd/pnfs.h        |  6 ++++++
>  fs/nfsd/state.h       |  5 ++---
>  4 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 898776c9d..749ace97e 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  		status = nfserr_jukebox;
>  		if (!ls)
>  			goto out;
> +		stid = &ls->ls_stid;
>  		mutex_lock(&ls->ls_mutex);
>  	} else {
>  		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
> @@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  			goto out_unlock_stid;
>  	}
>  
> +	/* A new or looked-up stateid may have been closed before ls_mutex. */
> +	spin_lock(&stid->sc_client->cl_lock);
> +	if (stid->sc_status & SC_STATUS_CLOSED) {
> +		spin_unlock(&stid->sc_client->cl_lock);
> +		status = nfserr_bad_stateid;
> +		goto out_unlock_stid;
> +	}
> +	spin_unlock(&stid->sc_client->cl_lock);
> +
>  	*lsp = ls;
>  	return 0;
>  
> @@ -537,12 +547,18 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
>  			nfs4_inc_and_copy_stateid(&lrp->lr_sid, &ls->ls_stid);
>  		lrp->lrs_present = true;
>  	} else {
> -		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> -		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
>  		lrp->lrs_present = false;
>  	}
>  	spin_unlock(&ls->ls_lock);
>  
> +	if (!lrp->lrs_present) {
> +		/* ls_mutex prevents a LAYOUTGET from adding another segment. */
> +		spin_lock(&ls->ls_stid.sc_client->cl_lock);
> +		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
> +		spin_unlock(&ls->ls_stid.sc_client->cl_lock);
> +		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> +	}
> +
>  	mutex_unlock(&ls->ls_mutex);
>  	nfs4_put_stid(&ls->ls_stid);
>  	nfsd4_free_layouts(&reaplist);
> @@ -594,6 +610,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
>  	spin_unlock(&ls->ls_lock);
>  }
>  
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +	LIST_HEAD(reaplist);
> +
> +	/*
> +	 * The caller holds a stateid reference until segment draining ends.
> +	 * Wait for any LAYOUTGET that passed the CLOSED check.
> +	 */
> +	mutex_lock(&ls->ls_mutex);
> +	nfsd4_return_all_layouts(ls, &reaplist);
> +	mutex_unlock(&ls->ls_mutex);
> +	nfsd4_free_layouts(&reaplist);
> +}
> +
>  void
>  nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 26d2d56a5..1740b86eb 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5873,9 +5873,21 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  		break;
>  	case SC_TYPE_LAYOUT:
>  		ls = layoutstateid(s);
> +		if (s->sc_status & SC_STATUS_CLOSED) {
> +			spin_unlock(&cl->cl_lock);
> +			break;
> +		}
> +		s->sc_status |= SC_STATUS_CLOSED;
> +		/*
> +		 * The IDR does not own a layout stateid reference. Keep ls
> +		 * alive while draining worker and segment references below;
> +		 * the final put releases this temporary reference.
> +		 */
> +		refcount_inc(&s->sc_count);
>  		list_del_init(&ls->ls_perclnt);
>  		spin_unlock(&cl->cl_lock);
>  		nfsd4_stop_layout_fence(ls);
> +		nfsd4_return_revoked_layout(ls);
>  		nfs4_put_stid(s);
>  		break;
>  	default:
> @@ -7901,6 +7913,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
>  		unsigned long id, tmp;
>  		struct nfs4_stid *stid;
>  
> +		if (clp->cl_minorversion != 0)
> +			continue;
>  		if (atomic_read(&clp->cl_admin_revoked) == 0)
>  			continue;
>  		if (is_client_expired(clp))
> diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
> index 559df0e5b..9b0f17a4d 100644
> --- a/fs/nfsd/pnfs.h
> +++ b/fs/nfsd/pnfs.h
> @@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
>  #ifdef CONFIG_NFSD_PNFS
>  void nfsd4_setup_layout_type(struct svc_export *exp);
>  void nfsd4_return_all_client_layouts(struct nfs4_client *);
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
>  void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp);
>  void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
> @@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
>  static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
>  }
> +
> +static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +}
> +
>  static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp)
>  {
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 79dfa47b8..63b581104 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -148,9 +148,8 @@ struct nfs4_stid {
>   * nn->deleg_lock protects sc_status for hashed delegation stateids.
>   * ->cl_lock protects the bits set as one is disposed of
>   * (SC_STATUS_CLOSED, SC_STATUS_FREEABLE, SC_STATUS_FREED) and
> - * sc_status for open and lock stateids. ->st_mutex also protects
> - * sc_status for open stateids. ->ls_lock protects sc_status for
> - * layout stateids.
> + * sc_status for open, lock, and layout stateids. ->st_mutex also protects
> + * sc_status for open stateids.
>   */
>  /*
>   * For an open stateid kept around *only* to process close replays.
> 
> base-commit: c807415bc69013a5f162264ee0e14f8742c182f7

Better! I do like the fact that we don't have different rules now.
Sashiko flagged a potential issue with v4.0 handling that needs to be
addressed, however:

https://sashiko.dev/#/patchset/20261013060021.46543-1-bint4b13%40gmail.com
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v3] nfsd: prevent a double put of revoked layout stateids

[Jinpyo Lee]

nfsd4_free_stateid() finds an admin-revoked layout stateid with
find_stateid_locked(), which does not acquire a reference. The layout
case drops cl_lock without marking the stateid closed, then stops fence
work and calls nfs4_put_stid() without owning a reference. Concurrent
disposal can therefore access a freed layout stateid or drop a reference
owned by another holder.

Mark the layout stateid CLOSED under cl_lock. Take a temporary reference
before dropping cl_lock and hold it while stopping fence work and
draining the remaining layout segments, then release it with the final
put. The IDR owns no layout reference.

Use cl_lock for the CLOSED update in LAYOUTRETURN as well, matching the
common stateid locking rule. Retain ls_mutex through that update and
recheck CLOSED under cl_lock after taking ls_mutex in LAYOUTGET, so a
stale LAYOUTGET cannot add a segment after the drain. Also make the newly
allocated layout stateid the error-path put target.

Make revoked-stateid disposal idempotent with SC_STATUS_FREED under
cl_lock. An in-flight reference can keep an admin-revoked OPEN, LOCK, or
DELEG stateid in the IDR after disposal. Restrict
nfs40_clean_admin_revoked() to v4.0 clients and skip FREED entries so its
retry loop cannot repeatedly select the same stateid or consume references
owned by other callers. Use FREED rather than CLOSED to guard disposal:
delegation recall rejection can set CLOSED before the first disposal call.

Reject FREED stateids in FREE_STATEID and TEST_STATEID as well, so a
stateid retained by an in-flight reference returns BAD_STATEID after
successful disposal instead of success or ADMIN_REVOKED.

Validation used nfsd-testing at c32fef6c476d. The patched x86_64 kernel
and modules were built with GCC 12.2 and booted with Generic KASAN,
lockdep, modular NFSD, and pNFS SCSI layout support. Basic NFSv4.2,
NFSv4.0, and NFSv3 read/write/unmount smoke tests passed.

Targeted runs forced concurrent two-slot FREE_STATEID, LAYOUTGET and
LAYOUTRETURN overlaps around ls_mutex, and an NFSv4.0 cleaner retry while
a READ held an OPEN stateid reference. Stateids were reaped once, stale
requests returned BAD_STATEID, and the NFSv4.0 cleaner did not process
the NFSv4.1 client. No KASAN, lockdep, refcount, list-corruption, Oops, or
kernel BUG report occurred; kernel taint remained zero.

The vulnerability research and validation were conducted by members of
the Tobabz team as part of the Best of the Best 15th program.

Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
Suggested-by: Chuck Lever <cel@kernel.org>
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
---
Changes in v3:

- Address the v4.0 disposal issue reported by Sashiko and forwarded by
  Jeff Layton: claim disposal with FREED under cl_lock and skip FREED
  stateids in the admin-revocation scan.
- Preserve first-time disposal of delegations already marked CLOSED, and
  reject FREED stateids in FREE_STATEID and TEST_STATEID.
- Rebase onto nfsd-testing c32fef6c476daa5830f600614657baee59c83339
  and add targeted runtime validation.

 fs/nfsd/nfs4layouts.c | 34 ++++++++++++++++++++++++++++++++--
 fs/nfsd/nfs4state.c   | 29 +++++++++++++++++++++++++----
 fs/nfsd/pnfs.h        |  6 ++++++
 fs/nfsd/state.h       |  6 +++---
 4 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 898776c9d..749ace97e 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 		status = nfserr_jukebox;
 		if (!ls)
 			goto out;
+		stid = &ls->ls_stid;
 		mutex_lock(&ls->ls_mutex);
 	} else {
 		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
@@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
 			goto out_unlock_stid;
 	}
 
+	/* A new or looked-up stateid may have been closed before ls_mutex. */
+	spin_lock(&stid->sc_client->cl_lock);
+	if (stid->sc_status & SC_STATUS_CLOSED) {
+		spin_unlock(&stid->sc_client->cl_lock);
+		status = nfserr_bad_stateid;
+		goto out_unlock_stid;
+	}
+	spin_unlock(&stid->sc_client->cl_lock);
+
 	*lsp = ls;
 	return 0;
 
@@ -537,12 +547,18 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
 			nfs4_inc_and_copy_stateid(&lrp->lr_sid, &ls->ls_stid);
 		lrp->lrs_present = true;
 	} else {
-		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
-		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
 		lrp->lrs_present = false;
 	}
 	spin_unlock(&ls->ls_lock);
 
+	if (!lrp->lrs_present) {
+		/* ls_mutex prevents a LAYOUTGET from adding another segment. */
+		spin_lock(&ls->ls_stid.sc_client->cl_lock);
+		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
+		spin_unlock(&ls->ls_stid.sc_client->cl_lock);
+		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
+	}
+
 	mutex_unlock(&ls->ls_mutex);
 	nfs4_put_stid(&ls->ls_stid);
 	nfsd4_free_layouts(&reaplist);
@@ -594,6 +610,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
 	spin_unlock(&ls->ls_lock);
 }
 
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+	LIST_HEAD(reaplist);
+
+	/*
+	 * The caller holds a stateid reference until segment draining ends.
+	 * Wait for any LAYOUTGET that passed the CLOSED check.
+	 */
+	mutex_lock(&ls->ls_mutex);
+	nfsd4_return_all_layouts(ls, &reaplist);
+	mutex_unlock(&ls->ls_mutex);
+	nfsd4_free_layouts(&reaplist);
+}
+
 void
 nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 26d2d56a5..0f1641eab 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5849,6 +5849,17 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 	struct nfs4_delegation *dp;
 	bool unhashed;
 
+	/*
+	 * In-flight references can keep a disposed stateid in the IDR.
+	 * CLOSED alone is not sufficient: a delegation may be closed
+	 * before its first call here.
+	 */
+	if (s->sc_status & SC_STATUS_FREED) {
+		spin_unlock(&cl->cl_lock);
+		return;
+	}
+	s->sc_status |= SC_STATUS_FREED;
+
 	switch (s->sc_type) {
 	case SC_TYPE_OPEN:
 		stp = openlockstateid(s);
@@ -5867,15 +5878,22 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
 	case SC_TYPE_DELEG:
 		dp = delegstateid(s);
 		list_del_init(&dp->dl_recall_lru);
-		s->sc_status |= SC_STATUS_FREED;
 		spin_unlock(&cl->cl_lock);
 		nfs4_put_stid(s);
 		break;
 	case SC_TYPE_LAYOUT:
 		ls = layoutstateid(s);
+		s->sc_status |= SC_STATUS_CLOSED;
+		/*
+		 * The IDR does not own a layout stateid reference. Keep ls
+		 * alive while draining worker and segment references below;
+		 * the final put releases this temporary reference.
+		 */
+		refcount_inc(&s->sc_count);
 		list_del_init(&ls->ls_perclnt);
 		spin_unlock(&cl->cl_lock);
 		nfsd4_stop_layout_fence(ls);
+		nfsd4_return_revoked_layout(ls);
 		nfs4_put_stid(s);
 		break;
 	default:
@@ -7901,6 +7919,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
 		unsigned long id, tmp;
 		struct nfs4_stid *stid;
 
+		if (clp->cl_minorversion != 0)
+			continue;
 		if (atomic_read(&clp->cl_admin_revoked) == 0)
 			continue;
 		if (is_client_expired(clp))
@@ -7908,7 +7928,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
 
 		spin_lock(&clp->cl_lock);
 		idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
-			if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
+			if ((stid->sc_status & SC_STATUS_ADMIN_REVOKED) &&
+			    !(stid->sc_status & SC_STATUS_FREED)) {
 				refcount_inc(&stid->sc_count);
 				atomic_inc(&clp->cl_rpc_users);
 				spin_unlock(&nn->client_lock);
@@ -8340,7 +8361,7 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
 		return status;
 	spin_lock(&cl->cl_lock);
 	s = find_stateid_locked(cl, stateid);
-	if (!s)
+	if (!s || s->sc_status & SC_STATUS_FREED)
 		goto out_unlock;
 	status = nfsd4_stid_check_stateid_generation(stateid, s, 1);
 	if (status)
@@ -8725,7 +8746,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	spin_lock(&cl->cl_lock);
 	s = find_stateid_locked(cl, stateid);
-	if (!s || s->sc_status & SC_STATUS_CLOSED)
+	if (!s || s->sc_status & (SC_STATUS_CLOSED | SC_STATUS_FREED))
 		goto out_unlock;
 	if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
 		nfsd4_drop_revoked_stid(s);
diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
index 559df0e5b..9b0f17a4d 100644
--- a/fs/nfsd/pnfs.h
+++ b/fs/nfsd/pnfs.h
@@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
 #ifdef CONFIG_NFSD_PNFS
 void nfsd4_setup_layout_type(struct svc_export *exp);
 void nfsd4_return_all_client_layouts(struct nfs4_client *);
+void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
 void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp);
 void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
@@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
 static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
 {
 }
+
+static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
+{
+}
+
 static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
 		struct nfs4_file *fp)
 {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 79dfa47b8..924a98af7 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -148,9 +148,8 @@ struct nfs4_stid {
  * nn->deleg_lock protects sc_status for hashed delegation stateids.
  * ->cl_lock protects the bits set as one is disposed of
  * (SC_STATUS_CLOSED, SC_STATUS_FREEABLE, SC_STATUS_FREED) and
- * sc_status for open and lock stateids. ->st_mutex also protects
- * sc_status for open stateids. ->ls_lock protects sc_status for
- * layout stateids.
+ * sc_status for open, lock, and layout stateids. ->st_mutex also protects
+ * sc_status for open stateids.
  */
 /*
  * For an open stateid kept around *only* to process close replays.
@@ -161,6 +160,7 @@ struct nfs4_stid {
 #define SC_STATUS_REVOKED	BIT(1)
 #define SC_STATUS_ADMIN_REVOKED	BIT(2)
 #define SC_STATUS_FREEABLE	BIT(3)
+/* Disposal has been claimed; in-flight references may still exist. */
 #define SC_STATUS_FREED		BIT(4)
 	unsigned short		sc_status;
 

base-commit: c32fef6c476daa5830f600614657baee59c83339

Re: [PATCH v3] nfsd: prevent a double put of revoked layout stateids

[Jeff Layton]

On Wed, 2026-10-14 at 15:12 +0900, Jinpyo Lee wrote:
> nfsd4_free_stateid() finds an admin-revoked layout stateid with
> find_stateid_locked(), which does not acquire a reference. The layout
> case drops cl_lock without marking the stateid closed, then stops fence
> work and calls nfs4_put_stid() without owning a reference. Concurrent
> disposal can therefore access a freed layout stateid or drop a reference
> owned by another holder.
> 
> Mark the layout stateid CLOSED under cl_lock. Take a temporary reference
> before dropping cl_lock and hold it while stopping fence work and
> draining the remaining layout segments, then release it with the final
> put. The IDR owns no layout reference.
> 
> Use cl_lock for the CLOSED update in LAYOUTRETURN as well, matching the
> common stateid locking rule. Retain ls_mutex through that update and
> recheck CLOSED under cl_lock after taking ls_mutex in LAYOUTGET, so a
> stale LAYOUTGET cannot add a segment after the drain. Also make the newly
> allocated layout stateid the error-path put target.
> 
> Make revoked-stateid disposal idempotent with SC_STATUS_FREED under
> cl_lock. An in-flight reference can keep an admin-revoked OPEN, LOCK, or
> DELEG stateid in the IDR after disposal. Restrict
> nfs40_clean_admin_revoked() to v4.0 clients and skip FREED entries so its
> retry loop cannot repeatedly select the same stateid or consume references
> owned by other callers. Use FREED rather than CLOSED to guard disposal:
> delegation recall rejection can set CLOSED before the first disposal call.
> 
> Reject FREED stateids in FREE_STATEID and TEST_STATEID as well, so a
> stateid retained by an in-flight reference returns BAD_STATEID after
> successful disposal instead of success or ADMIN_REVOKED.
> 
> Validation used nfsd-testing at c32fef6c476d. The patched x86_64 kernel
> and modules were built with GCC 12.2 and booted with Generic KASAN,
> lockdep, modular NFSD, and pNFS SCSI layout support. Basic NFSv4.2,
> NFSv4.0, and NFSv3 read/write/unmount smoke tests passed.
> 
> Targeted runs forced concurrent two-slot FREE_STATEID, LAYOUTGET and
> LAYOUTRETURN overlaps around ls_mutex, and an NFSv4.0 cleaner retry while
> a READ held an OPEN stateid reference. Stateids were reaped once, stale
> requests returned BAD_STATEID, and the NFSv4.0 cleaner did not process
> the NFSv4.1 client. No KASAN, lockdep, refcount, list-corruption, Oops, or
> kernel BUG report occurred; kernel taint remained zero.
> 
> The vulnerability research and validation were conducted by members of
> the Tobabz team as part of the Best of the Best 15th program.
> 
> Fixes: 86b9898920a6 ("NFSD: Handle layout stid in nfsd4_drop_revoked_stid()")
> Suggested-by: Chuck Lever <cel@kernel.org>
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Jinpyo Lee <bint4b13@gmail.com>
> ---
> Changes in v3:
> 
> - Address the v4.0 disposal issue reported by Sashiko and forwarded by
>   Jeff Layton: claim disposal with FREED under cl_lock and skip FREED
>   stateids in the admin-revocation scan.
> - Preserve first-time disposal of delegations already marked CLOSED, and
>   reject FREED stateids in FREE_STATEID and TEST_STATEID.
> - Rebase onto nfsd-testing c32fef6c476daa5830f600614657baee59c83339
>   and add targeted runtime validation.
> 
>  fs/nfsd/nfs4layouts.c | 34 ++++++++++++++++++++++++++++++++--
>  fs/nfsd/nfs4state.c   | 29 +++++++++++++++++++++++++----
>  fs/nfsd/pnfs.h        |  6 ++++++
>  fs/nfsd/state.h       |  6 +++---
>  4 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
> index 898776c9d..749ace97e 100644
> --- a/fs/nfsd/nfs4layouts.c
> +++ b/fs/nfsd/nfs4layouts.c
> @@ -312,6 +312,7 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  		status = nfserr_jukebox;
>  		if (!ls)
>  			goto out;
> +		stid = &ls->ls_stid;
>  		mutex_lock(&ls->ls_mutex);
>  	} else {
>  		ls = container_of(stid, struct nfs4_layout_stateid, ls_stid);
> @@ -324,6 +325,15 @@ nfsd4_preprocess_layout_stateid(struct svc_rqst *rqstp,
>  			goto out_unlock_stid;
>  	}
>  
> +	/* A new or looked-up stateid may have been closed before ls_mutex. */
> +	spin_lock(&stid->sc_client->cl_lock);
> +	if (stid->sc_status & SC_STATUS_CLOSED) {
> +		spin_unlock(&stid->sc_client->cl_lock);
> +		status = nfserr_bad_stateid;
> +		goto out_unlock_stid;
> +	}
> +	spin_unlock(&stid->sc_client->cl_lock);
> +
>  	*lsp = ls;
>  	return 0;
>  
> @@ -537,12 +547,18 @@ nfsd4_return_file_layouts(struct svc_rqst *rqstp,
>  			nfs4_inc_and_copy_stateid(&lrp->lr_sid, &ls->ls_stid);
>  		lrp->lrs_present = true;
>  	} else {
> -		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> -		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
>  		lrp->lrs_present = false;
>  	}
>  	spin_unlock(&ls->ls_lock);
>  
> +	if (!lrp->lrs_present) {
> +		/* ls_mutex prevents a LAYOUTGET from adding another segment. */
> +		spin_lock(&ls->ls_stid.sc_client->cl_lock);
> +		ls->ls_stid.sc_status |= SC_STATUS_CLOSED;
> +		spin_unlock(&ls->ls_stid.sc_client->cl_lock);
> +		trace_nfsd_layoutstate_unhash(&ls->ls_stid.sc_stateid);
> +	}
> +
>  	mutex_unlock(&ls->ls_mutex);
>  	nfs4_put_stid(&ls->ls_stid);
>  	nfsd4_free_layouts(&reaplist);
> @@ -594,6 +610,20 @@ nfsd4_return_all_layouts(struct nfs4_layout_stateid *ls,
>  	spin_unlock(&ls->ls_lock);
>  }
>  
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +	LIST_HEAD(reaplist);
> +
> +	/*
> +	 * The caller holds a stateid reference until segment draining ends.
> +	 * Wait for any LAYOUTGET that passed the CLOSED check.
> +	 */
> +	mutex_lock(&ls->ls_mutex);
> +	nfsd4_return_all_layouts(ls, &reaplist);
> +	mutex_unlock(&ls->ls_mutex);
> +	nfsd4_free_layouts(&reaplist);
> +}
> +
>  void
>  nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 26d2d56a5..0f1641eab 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5849,6 +5849,17 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  	struct nfs4_delegation *dp;
>  	bool unhashed;
>  
> +	/*
> +	 * In-flight references can keep a disposed stateid in the IDR.
> +	 * CLOSED alone is not sufficient: a delegation may be closed
> +	 * before its first call here.
> +	 */
> +	if (s->sc_status & SC_STATUS_FREED) {
> +		spin_unlock(&cl->cl_lock);
> +		return;
> +	}
> +	s->sc_status |= SC_STATUS_FREED;
> +
>  	switch (s->sc_type) {
>  	case SC_TYPE_OPEN:
>  		stp = openlockstateid(s);
> @@ -5867,15 +5878,22 @@ static void nfsd4_drop_revoked_stid(struct nfs4_stid *s)
>  	case SC_TYPE_DELEG:
>  		dp = delegstateid(s);
>  		list_del_init(&dp->dl_recall_lru);
> -		s->sc_status |= SC_STATUS_FREED;
>  		spin_unlock(&cl->cl_lock);
>  		nfs4_put_stid(s);
>  		break;
>  	case SC_TYPE_LAYOUT:
>  		ls = layoutstateid(s);
> +		s->sc_status |= SC_STATUS_CLOSED;
> +		/*
> +		 * The IDR does not own a layout stateid reference. Keep ls
> +		 * alive while draining worker and segment references below;
> +		 * the final put releases this temporary reference.
> +		 */
> +		refcount_inc(&s->sc_count);
>  		list_del_init(&ls->ls_perclnt);
>  		spin_unlock(&cl->cl_lock);
>  		nfsd4_stop_layout_fence(ls);
> +		nfsd4_return_revoked_layout(ls);
>  		nfs4_put_stid(s);
>  		break;
>  	default:
> @@ -7901,6 +7919,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
>  		unsigned long id, tmp;
>  		struct nfs4_stid *stid;
>  
> +		if (clp->cl_minorversion != 0)
> +			continue;
>  		if (atomic_read(&clp->cl_admin_revoked) == 0)
>  			continue;
>  		if (is_client_expired(clp))
> @@ -7908,7 +7928,8 @@ static void nfs40_clean_admin_revoked(struct nfsd_net *nn,
>  
>  		spin_lock(&clp->cl_lock);
>  		idr_for_each_entry_ul(&clp->cl_stateids, stid, tmp, id)
> -			if (stid->sc_status & SC_STATUS_ADMIN_REVOKED) {
> +			if ((stid->sc_status & SC_STATUS_ADMIN_REVOKED) &&
> +			    !(stid->sc_status & SC_STATUS_FREED)) {
>  				refcount_inc(&stid->sc_count);
>  				atomic_inc(&clp->cl_rpc_users);
>  				spin_unlock(&nn->client_lock);
> @@ -8340,7 +8361,7 @@ static __be32 nfsd4_validate_stateid(struct nfs4_client *cl, stateid_t *stateid)
>  		return status;
>  	spin_lock(&cl->cl_lock);
>  	s = find_stateid_locked(cl, stateid);
> -	if (!s)
> +	if (!s || s->sc_status & SC_STATUS_FREED)
>  		goto out_unlock;
>  	status = nfsd4_stid_check_stateid_generation(stateid, s, 1);
>  	if (status)
> @@ -8725,7 +8746,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	spin_lock(&cl->cl_lock);
>  	s = find_stateid_locked(cl, stateid);
> -	if (!s || s->sc_status & SC_STATUS_CLOSED)
> +	if (!s || s->sc_status & (SC_STATUS_CLOSED | SC_STATUS_FREED))
>  		goto out_unlock;
>  	if (s->sc_status & SC_STATUS_ADMIN_REVOKED) {
>  		nfsd4_drop_revoked_stid(s);
> diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
> index 559df0e5b..9b0f17a4d 100644
> --- a/fs/nfsd/pnfs.h
> +++ b/fs/nfsd/pnfs.h
> @@ -75,6 +75,7 @@ struct nfsd4_deviceid_map *nfsd4_find_devid_map(int idx);
>  #ifdef CONFIG_NFSD_PNFS
>  void nfsd4_setup_layout_type(struct svc_export *exp);
>  void nfsd4_return_all_client_layouts(struct nfs4_client *);
> +void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls);
>  void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp);
>  void nfsd4_close_layout(struct nfs4_layout_stateid *ls);
> @@ -93,6 +94,11 @@ static inline void nfsd4_setup_layout_type(struct svc_export *exp)
>  static inline void nfsd4_return_all_client_layouts(struct nfs4_client *clp)
>  {
>  }
> +
> +static inline void nfsd4_return_revoked_layout(struct nfs4_layout_stateid *ls)
> +{
> +}
> +
>  static inline void nfsd4_return_all_file_layouts(struct nfs4_client *clp,
>  		struct nfs4_file *fp)
>  {
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 79dfa47b8..924a98af7 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -148,9 +148,8 @@ struct nfs4_stid {
>   * nn->deleg_lock protects sc_status for hashed delegation stateids.
>   * ->cl_lock protects the bits set as one is disposed of
>   * (SC_STATUS_CLOSED, SC_STATUS_FREEABLE, SC_STATUS_FREED) and
> - * sc_status for open and lock stateids. ->st_mutex also protects
> - * sc_status for open stateids. ->ls_lock protects sc_status for
> - * layout stateids.
> + * sc_status for open, lock, and layout stateids. ->st_mutex also protects
> + * sc_status for open stateids.
>   */
>  /*
>   * For an open stateid kept around *only* to process close replays.
> @@ -161,6 +160,7 @@ struct nfs4_stid {
>  #define SC_STATUS_REVOKED	BIT(1)
>  #define SC_STATUS_ADMIN_REVOKED	BIT(2)
>  #define SC_STATUS_FREEABLE	BIT(3)
> +/* Disposal has been claimed; in-flight references may still exist. */
>  #define SC_STATUS_FREED		BIT(4)
>  	unsigned short		sc_status;
>  
> 
> base-commit: c32fef6c476daa5830f600614657baee59c83339

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3] nfsd: prevent a double put of revoked layout stateids

[Chuck Lever]

On Wed, 14 Oct 2026 15:12:18 +0900, Jinpyo Lee wrote:
> nfsd4_free_stateid() finds an admin-revoked layout stateid with
> find_stateid_locked(), which does not acquire a reference. The layout
> case drops cl_lock without marking the stateid closed, then stops fence
> work and calls nfs4_put_stid() without owning a reference. Concurrent
> disposal can therefore access a freed layout stateid or drop a reference
> owned by another holder.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/1] nfsd: prevent a double put of revoked layout stateids
      commit: d6f7f9b5c41466f3f54219f7968b26f2587794ee

--
Chuck Lever