Re: [PATCH 1/3] btrfs: unlock inode and extent on pages alloc failure in uring read

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/8/5 11:41, Yang Xiuwei 写道:
> btrfs_uring_read_extent() is only reached after btrfs_encoded_read()
> has taken the inode shared lock and the extent lock.  If allocating
> the pages array fails it returned -ENOMEM directly and left both
> locks held.
> 
> Fixes: 34310c442e17 ("btrfs: add io_uring command for encoded reads (ENCODED_READ ioctl)")
> Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
> ---
>   fs/btrfs/ioctl.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 68b33f365fda..c953e4409d5b 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -4626,8 +4626,10 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
>   
>   	nr_pages = DIV_ROUND_UP(disk_io_size, PAGE_SIZE);
>   	pages = kzalloc_objs(struct page *, nr_pages, GFP_NOFS);
> -	if (!pages)
> -		return -ENOMEM;
> +	if (!pages) {
> +		ret = -ENOMEM;
> +		goto out_fail;

The extra handling is only to unlock the inode and extent io tree.

And neither the inode nor extent io tree is locked by the function 
itself, but btrfs_encoded_read().

I do not believe this is the correct location to unlock inode/extent io 
tree.

Move the unlock into the caller, as the caller is already doing such 
unlock after copy_to_user() failed.

> +	}
>   	ret = btrfs_alloc_page_array(nr_pages, pages, GFP_NOFS);
>   	if (ret) {
>   		ret = -ENOMEM;
> @@ -4677,9 +4679,11 @@ static int btrfs_uring_read_extent(struct kiocb *iocb, struct iov_iter *iter,
>   	btrfs_unlock_extent(io_tree, start, lockend, &cached_state);
>   	btrfs_inode_unlock(inode, BTRFS_ILOCK_SHARED);
>   	kfree(priv);
> -	for (int i = 0; i < nr_pages; i++) {
> -		if (pages[i])
> -			__free_page(pages[i]);
> +	if (pages) {
> +		for (int i = 0; i < nr_pages; i++) {
> +			if (pages[i])
> +				__free_page(pages[i]);
> +		}
>   	}
>   	kfree(pages);
>   	return ret;