Re: [PATCH v4 7/9] NFSD: Prevent client use-after-free during blocked-lock reaping

On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
> A bare lock owner -- its only remaining reference a blocked lock on
> nn->blocked_locks_lru -- holds a raw pointer to its nfs4_client but
> no reference keeping the client alive. When the per-net laundromat
> reaps such a lock, freeing the nbl drops the owner reference
> held through flc_owner, and the final nfs4_put_stateowner()
> takes the client's cl_lock. Because the laundromat detaches the
> nbl first, __destroy_client() no longer finds it, so a concurrent
> force_expire_client() can free the client before nfs4_put_stateowner()
> runs, dereferencing cl_lock in freed memory.
> 
> Pin the client with cl_rpc_users before dropping
> nn->blocked_locks_lock, and skip clients already expiring, whose
> blocked locks __destroy_client() frees while holding an owner
> reference. Take nn->client_lock outside nn->blocked_locks_lock.
> Every other site holds nn->blocked_locks_lock as a leaf, acquiring
> no further lock, so placing nn->client_lock outside it cannot form
> a lock-order cycle.
> 
> Fixes: 7919d0a27f1e ("nfsd: add a LRU list for blocked locks")
> Signed-off-by: Chuck Lever <[email protected]>
> ---
>  fs/nfsd/nfs4state.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 142ba7d80539..4acd02f1642c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -357,6 +357,16 @@ free_blocked_lock(struct nfsd4_blocked_lock *nbl)
>  	kref_put(&nbl->nbl_kref, free_nbl);
>  }
>  
> +/* A blocked lock's flc_owner is its nfs4_lockowner. */
> +static struct nfs4_client *
> +nbl_client(struct nfsd4_blocked_lock *nbl)
> +{
> +	struct nfs4_lockowner *lo;
> +
> +	lo = (struct nfs4_lockowner *)nbl->nbl_lock.c.flc_owner;
> +	return lo->lo_owner.so_client;
> +}
> +
>  static void
>  remove_blocked_locks(struct nfs4_lockowner *lo)
>  {
> @@ -7591,22 +7601,29 @@ nfs4_laundromat(struct nfsd_net *nn)
>  	 * indefinitely once the lock does become free.
>  	 */
>  	BUG_ON(!list_empty(&reaplist));
> +	spin_lock(&nn->client_lock);
>  	spin_lock(&nn->blocked_locks_lock);
> -	while (!list_empty(&nn->blocked_locks_lru)) {
> -		nbl = list_first_entry(&nn->blocked_locks_lru,
> -					struct nfsd4_blocked_lock, nbl_lru);
> +	list_for_each_safe(pos, next, &nn->blocked_locks_lru) {
> +		nbl = list_entry(pos, struct nfsd4_blocked_lock, nbl_lru);
>  		if (!state_expired(&lt, nbl->nbl_time))
>  			break;
> +		clp = nbl_client(nbl);
> +		if (is_client_expired(clp))
> +			continue;
> +		atomic_inc(&clp->cl_rpc_users);
>  		list_move(&nbl->nbl_lru, &reaplist);
>  		list_del_init(&nbl->nbl_list);
>  	}
>  	spin_unlock(&nn->blocked_locks_lock);
> +	spin_unlock(&nn->client_lock);
>  
>  	while (!list_empty(&reaplist)) {
>  		nbl = list_first_entry(&reaplist,
>  					struct nfsd4_blocked_lock, nbl_lru);
> +		clp = nbl_client(nbl);
>  		list_del_init(&nbl->nbl_lru);
>  		free_blocked_lock(nbl);
> +		put_client_no_renew(clp);
>  	}
>  #ifdef CONFIG_NFSD_V4_2_INTER_SSC
>  	/* service the server-to-server copy delayed unmount list */

Reviewed-by: Jeff Layton <[email protected]>