Re: [PATCH] btrfs: fix wrong block_start calculation for btrfs_drop_extent_map_range()

[Greg KH <gregkh@linuxfoundation.org>]

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@vger.kernel.org # 6.7+
> > Fixes: c962098ca4af ("btrfs: fix incorrect splitting in btrfs_drop_extent_map_range")
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> $ git describe --contains c962098ca4af
> v6.5-rc7~4^2
> 
> so it should be
> CC: stable@vger.kernel.org # 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