On Mon, Apr 08, 2024 at 08:00:15AM +0800, Wang Yugui wrote:
> Hi,
>
> > [BUG]
> > During my extent_map cleanup/refactor, with more than too strict sanity
> > checks, extent-map-tests::test_case_7() would crash my extent_map sanity
> > checks.
> >
> > The problem is, after btrfs_drop_extent_map_range(), the resulted
> > extent_map has a @block_start way too large.
> > Meanwhile my btrfs_file_extent_item based members are returning a
> > correct @disk_bytenr along with correct @offset.
> >
> > The extent map layout looks like this:
> >
> > 0 16K 32K 48K
> > | PINNED | | Regular |
> >
> > The regular em at [32K, 48K) also has 32K @block_start.
> >
> > Then drop range [0, 36K), which should shrink the regular one to be
> > [36K, 48K).
> > However the @block_start is incorrect, we expect 32K + 4K, but got 52K.
> >
> > [CAUSE]
> > Inside btrfs_drop_extent_map_range() function, if we hit an extent_map
> > that covers the target range but is still beyond it, we need to split
> > that extent map into half:
> >
> > |<-- drop range -->|
> > |<----- existing extent_map --->|
> >
> > And if the extent map is not compressed, we need to forward
> > extent_map::block_start by the difference between the end of drop range
> > and the extent map start.
> >
> > However in that particular case, the difference is calculated using
> > (start + len - em->start).
> >
> > The problem is @start can be modified if the drop range covers any
> > pinned extent.
> >
> > This leads to wrong calculation, and would be caught by my later
> > extent_map sanity checks, which checks the em::block_start against
> > btrfs_file_extent_item::disk_bytenr + btrfs_file_extent_item::offset.
> >
> > And unfortunately this is going to cause data corruption, as the
> > splitted em is pointing an incorrect location, can cause either
> > unexpected read error or wild writes.
> >
> > [FIX]
> > Fix it by avoiding using @start completely, and use @end - em->start
> > instead, which @end is exclusive bytenr number.
> >
> > And update the test case to verify the @block_start to prevent such
> > problem from happening.
> >
> > CC: stable@xxxxxxxxxxxxxxx # 6.7+
> > Fixes: c962098ca4af ("btrfs: fix incorrect splitting in btrfs_drop_extent_map_range")
> > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>
> $ git describe --contains c962098ca4af
> v6.5-rc7~4^2
>
> so it should be
> CC: stable@xxxxxxxxxxxxxxx # 6.5+
As the "Fixes:" commit was backported to the following kernel releases:
6.1.47 6.4.12
it should go back to 6.1+ as well :)
But we can handle that when it hits Linus's tree.
thanks,
greg k-h
