Reviewed-by: Pavel Tikhomirov <[email protected]>

On 6/12/26 00:13, Konstantin Khorenko wrote:
> An asynchronous server-side NFSv4 COPY (struct nfsd4_copy) pins its
> source and destination files in nf_src/nf_dst. These references are
> dropped only by cleanup_async_copy(), which runs from the offload
> reaper (for OFFLOAD_DONE copies) and from the copy setup error path.
> 
> nfsd4_stop_copy() - used to abort a copy on OFFLOAD_CANCEL and during
> client teardown via nfsd4_shutdown_copy() - stops the copy kthread and
> drops the copy refcount, but never releases nf_src/nf_dst. The aborted
> copy has already been taken off clp->async_copies (cp_clp set to NULL),
> so the reaper can no longer reach it either. As a result every
> cancelled or torn-down async COPY leaks its two nfsd_file references.
> 
> When nfsd is later shut down ("nfsdctl threads 0" -> nfsd_destroy_serv
> -> nfsd_file_cache_shutdown()) the nfsd_file kmem_cache is destroyed
> while those leaked objects are still live:
> 
>   BUG nfsd_file: Objects remaining on __kmem_cache_shutdown()
>   WARNING: CPU: 3 PID: 849411 at mm/slub.c:1126 __slab_err
>    __kmem_cache_shutdown
>    kmem_cache_destroy
>    nfsd_file_cache_shutdown [nfsd]
>    nfsd_destroy_serv [nfsd]
>    nfsd_svc [nfsd]
>    nfsd_nl_threads_set_doit [nfsd]
> 
> which panics the box under panic_on_warn. Confirmed on the vmcore: the
> remaining nfsd_file objects are the nf_src/nf_dst of two in-flight async
> nfsd4_copy structures (cp_clp == NULL, refcount 1, files still held).
> 
> Release the files in nfsd4_stop_copy() and make release_copy_files()
> idempotent (NULL the pointers after the put) so it stays safe no matter
> which path - stop or reaper - reaches the copy.
> 
> This is the leak-fix portion of mainline commit 3daab3112f039 ("nfsd:
> cancel async COPY operations when admin revokes filesystem state"); the
> larger admin-revoke feature from that commit is not backported.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-134677
> Signed-off-by: Konstantin Khorenko <[email protected]>
> ---
>  fs/nfsd/nfs4proc.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 47dba8382ede..9b9ec637ff41 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1428,6 +1428,8 @@ static void nfs4_put_copy(struct nfsd4_copy *copy)
>       kfree(copy);
>  }
>  
> +static void release_copy_files(struct nfsd4_copy *copy);
> +
>  static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>  {
>       trace_nfsd_copy_async_cancel(copy);
> @@ -1436,6 +1438,15 @@ static void nfsd4_stop_copy(struct nfsd4_copy *copy)
>               copy->nfserr = nfs_ok;
>               set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
>       }
> +
> +     /*
> +      * The copy was removed from async_copies before this function
> +      * was called, so the reaper cannot clean it up. Release files
> +      * here regardless of who won the STOPPED race. If the thread
> +      * set STOPPED, it has finished using the files. If STOPPED
> +      * was set here, kthread_stop() waited for the thread to exit.
> +      */
> +     release_copy_files(copy);
>       nfs4_put_copy(copy);
>  }
>  
> @@ -1871,10 +1882,14 @@ static void dup_copy_fields(struct nfsd4_copy *src, 
> struct nfsd4_copy *dst)
>  
>  static void release_copy_files(struct nfsd4_copy *copy)
>  {
> -     if (copy->nf_src)
> +     if (copy->nf_src) {
>               nfsd_file_put(copy->nf_src);
> -     if (copy->nf_dst)
> +             copy->nf_src = NULL;
> +     }
> +     if (copy->nf_dst) {
>               nfsd_file_put(copy->nf_dst);
> +             copy->nf_dst = NULL;
> +     }
>  }
>  
>  static void cleanup_async_copy(struct nfsd4_copy *copy)

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to