diff mbox series

[v2] btrfs: subpage: fix the bitmap dump which can cause bitmap corruption

Message ID b93aac5fb44b46cd5391f3d5d177c0976e7d5ce0.1725001507.git.wqu@suse.com (mailing list archive)
State New
Headers show
Series [v2] btrfs: subpage: fix the bitmap dump which can cause bitmap corruption | expand

Commit Message

Qu Wenruo Aug. 30, 2024, 7:05 a.m. UTC
In commit 75258f20fb70 ("btrfs: subpage: dump extra subpage bitmaps for
debug") an internal macro GET_SUBPAGE_BITMAP() is introduced to grab the
bitmap of each attribute.

But that commit is using bitmap_cut() which will do the left shift of
the larger bitmap, causing incorrect values.

Thankfully this bitmap_cut() is only called for debug usage, and so far
it's not yet causing problem.

Fix it to use bitmap_read() to only grab the desired sub-bitmap.

Fixes: 75258f20fb70 ("btrfs: subpage: dump extra subpage bitmaps for debug")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Do not change the parameters of GET_SUBPAGE_BITMAP()
  To minimal the backport needed.
---
 fs/btrfs/subpage.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

David Sterba Aug. 30, 2024, 6:40 p.m. UTC | #1
On Fri, Aug 30, 2024 at 04:35:48PM +0930, Qu Wenruo wrote:
> In commit 75258f20fb70 ("btrfs: subpage: dump extra subpage bitmaps for
> debug") an internal macro GET_SUBPAGE_BITMAP() is introduced to grab the
> bitmap of each attribute.
> 
> But that commit is using bitmap_cut() which will do the left shift of
> the larger bitmap, causing incorrect values.
> 
> Thankfully this bitmap_cut() is only called for debug usage, and so far
> it's not yet causing problem.
> 
> Fix it to use bitmap_read() to only grab the desired sub-bitmap.
> 
> Fixes: 75258f20fb70 ("btrfs: subpage: dump extra subpage bitmaps for debug")
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>

> ---
> Changelog:
> v2:
> - Do not change the parameters of GET_SUBPAGE_BITMAP()
>   To minimal the backport needed.
> ---
>  fs/btrfs/subpage.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
> index 663f2f953a65..ca7d2aedfa8d 100644
> --- a/fs/btrfs/subpage.c
> +++ b/fs/btrfs/subpage.c
> @@ -870,9 +870,14 @@ void btrfs_folio_end_all_writers(const struct btrfs_fs_info *fs_info, struct fol
>  }
>  
>  #define GET_SUBPAGE_BITMAP(subpage, fs_info, name, dst)			\
> -	bitmap_cut(dst, subpage->bitmaps, 0,				\
> -		   fs_info->sectors_per_page * btrfs_bitmap_nr_##name,	\
> -		   fs_info->sectors_per_page)
> +{									\
> +	const int sectors_per_page = fs_info->sectors_per_page;		\
> +									\
> +	ASSERT(sectors_per_page < BITS_PER_LONG);			\
> +	*dst = bitmap_read(subpage->bitmaps,				\

Eventually this can be "dst = ...", the pointer was required for
bitmap_cut, but like that it's also acceptable.
diff mbox series

Patch

diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
index 663f2f953a65..ca7d2aedfa8d 100644
--- a/fs/btrfs/subpage.c
+++ b/fs/btrfs/subpage.c
@@ -870,9 +870,14 @@  void btrfs_folio_end_all_writers(const struct btrfs_fs_info *fs_info, struct fol
 }
 
 #define GET_SUBPAGE_BITMAP(subpage, fs_info, name, dst)			\
-	bitmap_cut(dst, subpage->bitmaps, 0,				\
-		   fs_info->sectors_per_page * btrfs_bitmap_nr_##name,	\
-		   fs_info->sectors_per_page)
+{									\
+	const int sectors_per_page = fs_info->sectors_per_page;		\
+									\
+	ASSERT(sectors_per_page < BITS_PER_LONG);			\
+	*dst = bitmap_read(subpage->bitmaps,				\
+			   sectors_per_page * btrfs_bitmap_nr_##name,	\
+			   sectors_per_page);				\
+}
 
 void __cold btrfs_subpage_dump_bitmap(const struct btrfs_fs_info *fs_info,
 				      struct folio *folio, u64 start, u32 len)