| Message ID | 99e2c01bb0366af9aec7522f7109c74b09c5509d.1676248644.git.wqu@suse.com |
|---|---|
| State | Accepted |
| Commit | 511a1303c9cf9663c7d4312e3a0693319f41095b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | fs: btrfs: limit the mapped length to the original length | expand |
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.
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 0x8c000000 /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 <schwab@linux-m68k.org> > Signed-off-by: Qu Wenruo <wqu@suse.com> Applied to u-boot/master, thanks!
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;
[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 0x8c000000 /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 <schwab@linux-m68k.org> Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/volumes.c | 2 ++ 1 file changed, 2 insertions(+)