Re: [PATCH v4 1/9] NFSD: Prevent lock owner use-after-free during client teardown

On 7/9/26 19:40, Chuck Lever wrote:
> __destroy_client() releases a client's open owners, but a lock owner
> whose only reference is a blocked lock (nbl) stays on
> cl_ownerstr_hashtbl.  client_has_state() does not count a bare owner,
> so DESTROY_CLIENTID can reach __destroy_client() with such owners
> present.
>
> [...]
>
> Reported-by: Wolfgang Walter <[email protected]>
> Closes: https://lore.kernel.org/linux-nfs/[email protected]/
> Fixes: 68ef3bc31664 ("nfsd: remove blocked locks on client teardown")

What's the plan to merge this change? From the outside it looks like a
fix for a user reported regression from this merge window, as Wolfgang
afaics blamed commit 516403d4d85607 ("Revert "NFSD: Defer sub-object
cleanup in export put callbacks""), which also made it to various stable
trees.

So shouldn't this change ideally be on it's way to Linus by now? Ideally
with a stable tag to ensure quick backporting? Or is it not a good idea
to have this in without the follow-up patches from this series?

Or did I get something totally wrong (which is easily possible from my
external point)?

Ciao, Thorsten


> Reviewed-by: NeilBrown <[email protected]>
> Reviewed-by: Jeff Layton <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  fs/nfsd/nfs4state.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a4398dc861a5..e000ed3e96e9 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2758,14 +2758,24 @@ __destroy_client(struct nfs4_client *clp)
>  		release_openowner(oo);
>  	}
>  	for (i = 0; i < OWNER_HASH_SIZE; i++) {
> -		struct nfs4_stateowner *so, *tmp;
> +		struct nfs4_stateowner *so;
>  
> -		list_for_each_entry_safe(so, tmp, &clp->cl_ownerstr_hashtbl[i],
> -					 so_strhash) {
> +		spin_lock(&clp->cl_lock);
> +		while (!list_empty(&clp->cl_ownerstr_hashtbl[i])) {
> +			so = list_first_entry(&clp->cl_ownerstr_hashtbl[i],
> +					      struct nfs4_stateowner, so_strhash);
>  			/* Should be no openowners at this point */
>  			WARN_ON_ONCE(so->so_is_open_owner);
> +			nfs4_get_stateowner(so);
> +			unhash_lockowner_locked(lockowner(so));
> +			spin_unlock(&clp->cl_lock);
> +
>  			remove_blocked_locks(lockowner(so));
> +			nfs4_put_stateowner(so);
> +
> +			spin_lock(&clp->cl_lock);
>  		}
> +		spin_unlock(&clp->cl_lock);
>  	}
>  	nfsd4_return_all_client_layouts(clp);
>  	nfsd4_shutdown_copy(clp);
>