Re: [PATCH v2] btrfs: fix extent map leak in NOCOW direct I/O write

[Qu Wenruo <wqu@suse.com>]

在 2026/7/5 15:16, Shuangpeng Bai 写道:
> btrfs_dio_iomap_begin() calls btrfs_get_extent(), which returns an
> extent map reference that must be dropped on all exit paths.
> 
> For direct writes into a NOCOW range, btrfs_get_blocks_direct_write()
> keeps using that extent map and asks btrfs_create_dio_extent() to
> allocate the ordered extent. If that fails, for example because
> btrfs_alloc_ordered_extent() fails, the function returns the error
> without dropping the input extent map. The PREALLOC path avoided this by
> dropping the input extent map before replacing it with the newly
> created one.
> 
> Check the error from btrfs_create_dio_extent() before replacing the
> map and drop the input extent map on failure.
> 
> Fixes: 5f9a8a51d8b9 ("Btrfs: add semaphore to synchronize direct IO writes with fsync")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
> Changes since v1:
> - Add a comment explaining the returned @em2 pointer.
> - Use @em2 to decide whether to replace the old extent map and assert
>    that this only happens for PREALLOC writes.
> 
>   fs/btrfs/direct-io.c | 19 +++++++++++++------
>   1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index 460326d34143..19a1259b3b2f 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -281,17 +281,24 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
>   		em2 = btrfs_create_dio_extent(BTRFS_I(inode), dio_data, start,
>   					      &file_extent, type);
>   		btrfs_dec_nocow_writers(bg);
> -		if (type == BTRFS_ORDERED_PREALLOC) {
> -			btrfs_free_extent_map(em);
> -			*map = em2;
> -			em = em2;
> -		}
> -
>   		if (IS_ERR(em2)) {
>   			ret = PTR_ERR(em2);
> +			btrfs_free_extent_map(em);
> +			*map = NULL;
>   			goto out;
>   		}
>   
> +		/*
> +		 * True NOCOW writes don't need to create a new extent map,
> +		 * while PREALLOC writes must replace the existing one.
> +		 */
> +		if (em2) {
> +			ASSERT(type == BTRFS_ORDERED_PREALLOC);
> +			btrfs_free_extent_map(em);
> +			*map = em2;
> +			em = em2;
> +		}
> +
>   		dio_data->nocow_done = true;
>   	} else {
>   		/* Our caller expects us to free the input extent map. */