Re: [PATCH] fs: btrfs: limit the mapped length to the original length
On Mon, Feb 13, 2023 at 08:37:59AM +0800, Qu Wenruo wrote:
> [BUG]
> There is a bug report that btrfs driver caused hang during file read:
>
> This breaks btrfs on the HiFive Unmatched.
>
> => pci enum
> PCIE-0: Link up (Gen1-x8, Bus0)
> => nvme scan
> => load nvme 0:2 0x8c00 /boot/dtb/sifive/hifive-unmatched-a00.dtb
> [hangs]
>
> [CAUSE]
> The reporter provided some debug output:
>
> read_extent_data: cur=615817216, orig_len=16384, cur_len=16384
> read_extent_data: btrfs_map_block: cur_len=479944704; ret=0
> read_extent_data: ret=0
> read_extent_data: cur=615833600, orig_len=4096, cur_len=4096
> read_extent_data: btrfs_map_block: cur_len=479928320; ret=0
>
> Note the second and the last line, the @cur_len is 450+MiB, which is
> almost a chunk size.
>
> And inside __btrfs_map_block(), we limits the returned value to stripe
> length, but that's depending on the chunk type:
>
> if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
>BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4 |
>BTRFS_BLOCK_GROUP_RAID5 | BTRFS_BLOCK_GROUP_RAID6 |
>BTRFS_BLOCK_GROUP_RAID10 |
>BTRFS_BLOCK_GROUP_DUP)) {
> /* we limit the length of each bio to what fits in a stripe */
> *length = min_t(u64, ce->size - offset,
> map->stripe_len - stripe_offset);
> } else {
> *length = ce->size - offset;
> }
>
> This means, if the chunk is SINGLE profile, then we don't limit the
> returned length at all, and even for other profiles, we can still return
> a length much larger than the requested one.
>
> [FIX]
> Properly clamp the returned length, preventing it from returning a much
> larger range than expected.
>
> Reported-by: Andreas Schwab
> Signed-off-by: Qu Wenruo
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] fs: btrfs: limit the mapped length to the original length
On Feb 13 2023, Qu Wenruo wrote:
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 4aaaeab663f5..7d4095d9ca88 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -956,6 +956,7 @@ int __btrfs_map_block(struct btrfs_fs_info *fs_info, int
> rw,
> struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
> struct cache_extent *ce;
> struct map_lookup *map;
> + u64 orig_len = *length;
> u64 offset;
> u64 stripe_offset;
> u64 *raid_map = NULL;
> @@ -1047,6 +1048,7 @@ again:
> } else {
> *length = ce->size - offset;
> }
> + *length = min_t(u64, *length, orig_len);
>
> if (!multi_ret)
> goto out;
I can confirm that this fixes the issue.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
[PATCH] fs: btrfs: limit the mapped length to the original length
[BUG]
There is a bug report that btrfs driver caused hang during file read:
This breaks btrfs on the HiFive Unmatched.
=> pci enum
PCIE-0: Link up (Gen1-x8, Bus0)
=> nvme scan
=> load nvme 0:2 0x8c00 /boot/dtb/sifive/hifive-unmatched-a00.dtb
[hangs]
[CAUSE]
The reporter provided some debug output:
read_extent_data: cur=615817216, orig_len=16384, cur_len=16384
read_extent_data: btrfs_map_block: cur_len=479944704; ret=0
read_extent_data: ret=0
read_extent_data: cur=615833600, orig_len=4096, cur_len=4096
read_extent_data: btrfs_map_block: cur_len=479928320; ret=0
Note the second and the last line, the @cur_len is 450+MiB, which is
almost a chunk size.
And inside __btrfs_map_block(), we limits the returned value to stripe
length, but that's depending on the chunk type:
if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4 |
BTRFS_BLOCK_GROUP_RAID5 | BTRFS_BLOCK_GROUP_RAID6 |
BTRFS_BLOCK_GROUP_RAID10 |
BTRFS_BLOCK_GROUP_DUP)) {
/* we limit the length of each bio to what fits in a stripe */
*length = min_t(u64, ce->size - offset,
map->stripe_len - stripe_offset);
} else {
*length = ce->size - offset;
}
This means, if the chunk is SINGLE profile, then we don't limit the
returned length at all, and even for other profiles, we can still return
a length much larger than the requested one.
[FIX]
Properly clamp the returned length, preventing it from returning a much
larger range than expected.
Reported-by: Andreas Schwab
Signed-off-by: Qu Wenruo
---
fs/btrfs/volumes.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4aaaeab663f5..7d4095d9ca88 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -956,6 +956,7 @@ int __btrfs_map_block(struct btrfs_fs_info *fs_info, int rw,
struct btrfs_mapping_tree *map_tree = &fs_info->mapping_tree;
struct cache_extent *ce;
struct map_lookup *map;
+ u64 orig_len = *length;
u64 offset;
u64 stripe_offset;
u64 *raid_map = NULL;
@@ -1047,6 +1048,7 @@ again:
} else {
*length = ce->size - offset;
}
+ *length = min_t(u64, *length, orig_len);
if (!multi_ret)
goto out;
--
2.39.1
