Re: [PATCH] btrfs: don't let shrinker touch extent_maps that are being logged

[Filipe Manana]

On Mon, Jun 29, 2026 at 2:41 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> The extent map shrinker can free an extent map that is still owned by an
> in-flight fsync and still linked on the inode's modified_extents list,
> corrupting that list and eventually causing an RCU stall.
>
> btrfs_scan_inode() currently skips EXTENT_FLAG_PINNED maps, then calls
> btrfs_remove_extent_mapping() followed by btrfs_free_extent_map():
>
>         if (em->flags & EXTENT_FLAG_PINNED)
>                 goto next;
>         ...
>         btrfs_remove_extent_mapping(inode, em);
>         btrfs_free_extent_map(em);
>
> But btrfs_remove_extent_mapping() deliberately does NOT unlink a map that
> has EXTENT_FLAG_LOGGING set:
>
>         if (!(em->flags & EXTENT_FLAG_LOGGING))
>                 list_del_init(&em->list);
>         remove_em(inode, em);
>
> This sets up a UAF situation where a later fsync() can trip over the
> now-freed extent_map still on the modified_extents() list.

I don't see how that can happen, because fsync always takes the
inode's i_mmap_lock (in write/exclusive mode), and btrfs_scan_inode()
takes the same lock too (in shared/read mode).
EXTENT_FLAG_LOGGING is only set and cleared during fsync, so how can
the shrinker race with fsync?

Also, there should be no () after modified_extents (it's not a function name).

Thanks.

>
> Fix it by having the shrinker skip maps that are being logged, the same
> way it skips pinned maps. Such a map is owned by the in-flight fsync and
> will become reclaimable again once logging clears the flag.
>
> Fixes: 956a17d9d050 ("btrfs: add a shrinker for extent maps")
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> We've started hitting a number of these problems in our fleet. It
> seems to mostly happen on ARM64 architecture, but there have been some
> WARN_ONs that popped on x86_64 too.
> ---
>  fs/btrfs/extent_map.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index fce9c5cc0122..128f7800e101 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -1166,7 +1166,13 @@ static long btrfs_scan_inode(struct btrfs_inode *inode, struct btrfs_em_shrink_c
>                 em = rb_entry(node, struct extent_map, rb_node);
>                 ctx->scanned++;
>
> -               if (em->flags & EXTENT_FLAG_PINNED)
> +               /*
> +                * Skip extent maps that are pinned or are being logged. The
> +                * i_mmap_lock should prevent this from seeing LOGGING on extent_maps
> +                * directly associated with inode, but em may be associated with
> +                * other, dependent inodes and their locks are not held.
> +                */
> +               if (em->flags & (EXTENT_FLAG_PINNED | EXTENT_FLAG_LOGGING))
>                         goto next;
>
>                 /*
>
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260629-btrfs-skip-logging-3e31701d9647
>
> Best regards,
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
>