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

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

[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>
---
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. */
-- 
2.43.0

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

[Qu Wenruo]

在 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. */