Re: [RFC PATCH 2/7] NFSD: Limit the number of concurrent async COPY operations

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2024-08-28 at 13:40 -0400, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Nothing appears to limit the number of concurrent async COPY
> operations that clients can start. In addition, AFAICT each async
> COPY can copy an unlimited number of 4MB chunks, so can run for a
> long time. Thus IMO async COPY can become a DoS vector.
> 
> Add a restriction mechanism that bounds the number of concurrent
> background COPY operations. Start simple and try to be fair -- this
> patch implements a per-namespace limit.
> 
> An async COPY request that occurs while this limit is exceeded gets
> NFS4ERR_DELAY. The requesting client can choose to send the request
> again after a delay or fall back to a traditional read/write style
> copy.
> 
> If there is need to make the mechanism more sophisticated, we can
> visit that in future patches.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/netns.h     |  1 +
>  fs/nfsd/nfs4proc.c  | 12 ++++++++++--
>  fs/nfsd/nfs4state.c |  1 +
>  fs/nfsd/xdr4.h      |  1 +
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index 14ec15656320..5cae26917436 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -148,6 +148,7 @@ struct nfsd_net {
>  	u32		s2s_cp_cl_id;
>  	struct idr	s2s_cp_stateids;
>  	spinlock_t	s2s_cp_lock;
> +	atomic_t	pending_async_copies;
>  
>  	/*
>  	 * Version information
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 60c526adc27c..27f7eceb3b00 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1279,6 +1279,7 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
>  {
>  	if (!refcount_dec_and_test(&copy->refcount))
>  		return;
> +	atomic_dec(&copy->cp_nn->pending_async_copies);
>  	kfree(copy->cp_src);
>  	kfree(copy);
>  }
> @@ -1833,10 +1834,17 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
>  		sizeof(struct knfsd_fh));
>  	if (nfsd4_copy_is_async(copy)) {
> -		status = nfserrno(-ENOMEM);
> +		/* Arbitrary cap on number of pending async copy operations */
> +		int nrthreads = atomic_read(&rqstp->rq_pool->sp_nrthreads);
> +
>  		async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
>  		if (!async_copy)
>  			goto out_err;
> +		async_copy->cp_nn = nn;
> +		if (atomic_inc_return(&nn->pending_async_copies) > nrthreads) {
> +			atomic_dec(&nn->pending_async_copies);
> +			goto out_err;
> +		}
>  		INIT_LIST_HEAD(&async_copy->copies);
>  		refcount_set(&async_copy->refcount, 1);
>  		async_copy->cp_src = kmalloc(sizeof(*async_copy->cp_src), GFP_KERNEL);
> @@ -1876,7 +1884,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	}
>  	if (async_copy)
>  		cleanup_async_copy(async_copy);
> -	status = nfserrno(-ENOMEM);
> +	status = nfserr_jukebox;

ENOMEM gets translated to nfserr_jukebox anyway, so this doesn't change
anything (good).

>  	goto out;
>  }
>  
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a20c2c9d7d45..aaebc60cc77c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -8554,6 +8554,7 @@ static int nfs4_state_create_net(struct net *net)
>  	spin_lock_init(&nn->client_lock);
>  	spin_lock_init(&nn->s2s_cp_lock);
>  	idr_init(&nn->s2s_cp_stateids);
> +	atomic_set(&nn->pending_async_copies, 0);
>  
>  	spin_lock_init(&nn->blocked_locks_lock);
>  	INIT_LIST_HEAD(&nn->blocked_locks_lru);
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index fbdd42cde1fa..2a21a7662e03 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -713,6 +713,7 @@ struct nfsd4_copy {
>  	struct nfsd4_ssc_umount_item *ss_nsui;
>  	struct nfs_fh		c_fh;
>  	nfs4_stateid		stateid;
> +	struct nfsd_net		*cp_nn;
>  };
>  
>  static inline void nfsd4_copy_set_sync(struct nfsd4_copy *copy, bool sync)

A per-ns limit sounds like a reasonable place to start.

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