Re: [RFC PATCH v1 7/8] nfs: allow LOCALIO for P2PDMA pages only as aligned direct I/O

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: memory and refcount leak in nfs_local_doio() on initialization failure
--

commit 9173ddb2cde9be449dc0867c1db4182cfbf16690
Author: Pranjal Shrivastava <praan@google.com>

nfs: allow LOCALIO for P2PDMA pages only as aligned direct I/O
    
LOCALIO hands READs and WRITEs to the local filesystem, which can take
P2PDMA pages only if its disk can DMA to them. Use LOCALIO for such I/O
only if the filesystem's block device supports P2PDMA. Otherwise send a
regular READ or WRITE RPC, as if LOCALIO were off.

[ ... ]

> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index ec535ed3051d4..2d1d07c460fd4 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -1023,6 +1023,9 @@ static int nfs_generic_pg_pgios(struct nfs_pageio_descriptor *desc)
>  					  &hdr->args.context->nfl,
>  					  hdr->args.context->mode);
>  
> +		if (localio && hdr->args.p2pdma)
> +			localio = nfs_local_p2pdma_check(localio);
> +
>  		if (NFS_SERVER(hdr->inode)->nfs_client->cl_minorversion)
>  			task_flags = RPC_TASK_MOVEABLE;
>  		ret = nfs_initiate_pgio(NFS_CLIENT(hdr->inode),
>  					hdr,
>  					hdr->cred,
>  					NFS_PROTO(hdr->inode),
>  					desc->pg_rpc_callops,
>  					desc->pg_ioflags,
>  					RPC_TASK_CRED_NOREF | task_flags,
>  					localio);

[Severity: High]
Does this code leak the localio reference and the nfs_pgio_header on
initialization failure?

The localio reference is created here and passed down. On error return from
nfs_initiate_pgio() -> nfs_local_doio(), this caller does not drop the
reference, nor does it have access to the requests bound to the leaked hdr.

If nfs_local_iocb_init() fails (e.g. -ENOMEM) or !hdr->args.count evaluates
to true, nfs_local_doio() returns early:

fs/nfs/localio.c:nfs_local_doio() {
    ...
	if (!hdr->args.count)
		return 0;

	iocb = nfs_local_iocb_init(hdr, localio);
	if (IS_ERR(iocb))
		return PTR_ERR(iocb);
    ...
}

In these early return paths, it looks like the nfsd_file reference (localio)
is leaked because nfs_local_file_put(localio) is not called.

Furthermore, call_ops->rpc_release(hdr) is never invoked. Because the
requests (nfs_page structs) were already moved to hdr->pages in the caller
(nfs_generic_pgio), the nfs_pgio_header and all attached pages seem to be
permanently leaked, potentially resulting in hung processes waiting on page
writeback/read and memory depletion.

By comparison, nfs_local_commit() correctly invokes
nfs_local_release_commit_data() to clean up on its allocation failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006233248.705086-1-praan@google.com?part=7