| Message ID | 90e84962486d7ab5a8bca92e329fe3ee6864680f.1666312963.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: btrfs: don't trust sub_stripes from disk | expand |
On 2022/10/21 08:44, Qu Wenruo wrote: > [BUG] > There are two reports (the earliest one from LKP, a more recent one from > kernel bugzilla) that we can have some chunks with 0 as sub_stripes. > > This will cause divide-by-zero errors at btrfs_rmap_block, which is > introduced by a recent kernel patch ac0677348f3c ("btrfs: merge > calculations for simple striped profiles in btrfs_rmap_block"): > > if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | > BTRFS_BLOCK_GROUP_RAID10)) { > stripe_nr = stripe_nr * map->num_stripes + i; > stripe_nr = div_u64(stripe_nr, map->sub_stripes); <<< > } > > [CAUSE] > From the more recent report, it has been proven that we have some chunks > with 0 as sub_stripes, mostly caused by older mkfs. > > It turns out that the mkfs.btrfs fix is only introduced in 6718ab4d33aa > ("btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk") > which is included in v5.4 btrfs-progs release. > > So there would be quite some old fses with such 0 sub_stripes. > > [FIX] > Just don't trust the sub_stripes values from disk. > > We have a trusted btrfs_raid_array[] to fetch the correct sub_stripes > numbers for each profile. > > By this, we can keep the compatibility with older fses while still avoid > divide-by-zero bugs. > > Fixes: ac0677348f3c ("btrfs: merge calculations for simple striped profiles in btrfs_rmap_block") > Reported-by: kernel test robot <oliver.sang@intel.com> > Reported-by: Viktor Kuzmin <kvaster@gmail.com> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216559 > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- LGTM, Reviewed-by: Su Yue <glass@fydeos.io> > fs/btrfs/volumes.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 94ba46d57920..39588cb9a7b6 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -7142,6 +7142,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, > u64 devid; > u64 type; > u8 uuid[BTRFS_UUID_SIZE]; > + int index; > int num_stripes; > int ret; > int i; > @@ -7149,6 +7150,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, > logical = key->offset; > length = btrfs_chunk_length(leaf, chunk); > type = btrfs_chunk_type(leaf, chunk); > + index = btrfs_bg_flags_to_raid_index(type); > num_stripes = btrfs_chunk_num_stripes(leaf, chunk); > > #if BITS_PER_LONG == 32 > @@ -7202,7 +7204,14 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, > map->io_align = btrfs_chunk_io_align(leaf, chunk); > map->stripe_len = btrfs_chunk_stripe_len(leaf, chunk); > map->type = type; > - map->sub_stripes = btrfs_chunk_sub_stripes(leaf, chunk); > + /* > + * Don't trust the sub_stripes value, as for profiles other > + * than RAID10, they may have 0 as sub_stripes for older mkfs. > + * In that case, it can cause divide-by-zero errors later. > + * Since currently sub_stripes is fixed for each profile, let's > + * use the trusted value instead. > + */ > + map->sub_stripes = btrfs_raid_array[index].sub_stripes; > map->verified_stripes = 0; > em->orig_block_len = btrfs_calc_stripe_length(em); > for (i = 0; i < num_stripes; i++) {
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Fri, Oct 21, 2022 at 08:43:45AM +0800, Qu Wenruo wrote: > [BUG] > There are two reports (the earliest one from LKP, a more recent one from > kernel bugzilla) that we can have some chunks with 0 as sub_stripes. > > This will cause divide-by-zero errors at btrfs_rmap_block, which is > introduced by a recent kernel patch ac0677348f3c ("btrfs: merge > calculations for simple striped profiles in btrfs_rmap_block"): > > if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | > BTRFS_BLOCK_GROUP_RAID10)) { > stripe_nr = stripe_nr * map->num_stripes + i; > stripe_nr = div_u64(stripe_nr, map->sub_stripes); <<< > } > > [CAUSE] > >From the more recent report, it has been proven that we have some chunks > with 0 as sub_stripes, mostly caused by older mkfs. > > It turns out that the mkfs.btrfs fix is only introduced in 6718ab4d33aa > ("btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk") > which is included in v5.4 btrfs-progs release. > > So there would be quite some old fses with such 0 sub_stripes. > > [FIX] > Just don't trust the sub_stripes values from disk. > > We have a trusted btrfs_raid_array[] to fetch the correct sub_stripes > numbers for each profile. > > By this, we can keep the compatibility with older fses while still avoid > divide-by-zero bugs. > > Fixes: ac0677348f3c ("btrfs: merge calculations for simple striped profiles in btrfs_rmap_block") > Reported-by: kernel test robot <oliver.sang@intel.com> > Reported-by: Viktor Kuzmin <kvaster@gmail.com> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216559 > Signed-off-by: Qu Wenruo <wqu@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 94ba46d57920..39588cb9a7b6 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -7142,6 +7142,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, u64 devid; u64 type; u8 uuid[BTRFS_UUID_SIZE]; + int index; int num_stripes; int ret; int i; @@ -7149,6 +7150,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, logical = key->offset; length = btrfs_chunk_length(leaf, chunk); type = btrfs_chunk_type(leaf, chunk); + index = btrfs_bg_flags_to_raid_index(type); num_stripes = btrfs_chunk_num_stripes(leaf, chunk); #if BITS_PER_LONG == 32 @@ -7202,7 +7204,14 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, map->io_align = btrfs_chunk_io_align(leaf, chunk); map->stripe_len = btrfs_chunk_stripe_len(leaf, chunk); map->type = type; - map->sub_stripes = btrfs_chunk_sub_stripes(leaf, chunk); + /* + * Don't trust the sub_stripes value, as for profiles other + * than RAID10, they may have 0 as sub_stripes for older mkfs. + * In that case, it can cause divide-by-zero errors later. + * Since currently sub_stripes is fixed for each profile, let's + * use the trusted value instead. + */ + map->sub_stripes = btrfs_raid_array[index].sub_stripes; map->verified_stripes = 0; em->orig_block_len = btrfs_calc_stripe_length(em); for (i = 0; i < num_stripes; i++) {
[BUG] There are two reports (the earliest one from LKP, a more recent one from kernel bugzilla) that we can have some chunks with 0 as sub_stripes. This will cause divide-by-zero errors at btrfs_rmap_block, which is introduced by a recent kernel patch ac0677348f3c ("btrfs: merge calculations for simple striped profiles in btrfs_rmap_block"): if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID10)) { stripe_nr = stripe_nr * map->num_stripes + i; stripe_nr = div_u64(stripe_nr, map->sub_stripes); <<< } [CAUSE] From the more recent report, it has been proven that we have some chunks with 0 as sub_stripes, mostly caused by older mkfs. It turns out that the mkfs.btrfs fix is only introduced in 6718ab4d33aa ("btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk") which is included in v5.4 btrfs-progs release. So there would be quite some old fses with such 0 sub_stripes. [FIX] Just don't trust the sub_stripes values from disk. We have a trusted btrfs_raid_array[] to fetch the correct sub_stripes numbers for each profile. By this, we can keep the compatibility with older fses while still avoid divide-by-zero bugs. Fixes: ac0677348f3c ("btrfs: merge calculations for simple striped profiles in btrfs_rmap_block") Reported-by: kernel test robot <oliver.sang@intel.com> Reported-by: Viktor Kuzmin <kvaster@gmail.com> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216559 Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/volumes.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)