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

[Filipe Manana]

On Sun, Jul 5, 2026 at 6:47 AM Shuangpeng Bai
<shuangpeng.kernel@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Shuangpeng Bai <shuangpeng.kernel@xxxxxxxxx>

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Adding it to the for-next branch, thanks.

> ---
> 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
>