| Message ID | 20181005082525.22441-2-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enhance btrfs_verify_dev_extents() to do more checks on dev extents | expand |
On 5.10.2018 11:25, Qu Wenruo wrote: > Enhance btrfs_verify_dev_extents() to remember previous checked dev > extents, so it can check if one dev extent overlaps with previous dev extent. > > Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> > Signed-off-by: Qu Wenruo <wqu@suse.com> If this is related to the initial report here: https://lore.kernel.org/linux-btrfs/782f6000-30c0-0085-abd2-74ec5827c903@mendix.com/T/#m609ccb5d32998e8ba5cfa9901c1ab56a38a6f374 I'd rather have it as a link to the commit: Link: https://lore.kernel.org/linux-btrfs/b3461a38-e5f8-f41d-c67c-2efac8129054@mendix.com/ Since I wouldn't expect this occur but clearly there is a bug. > --- > fs/btrfs/volumes.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index da86706123ff..bf0b2c16847a 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -7462,6 +7462,8 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) > struct btrfs_path *path; > struct btrfs_root *root = fs_info->dev_root; > struct btrfs_key key; > + u64 prev_devid = 0; > + u64 prev_dev_ext_end = 0; > int ret = 0; > > key.objectid = 1; > @@ -7506,10 +7508,22 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) > chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dext); > physical_len = btrfs_dev_extent_length(leaf, dext); > > + /* Check if this dev extent over lap with previous one */ > + if (devid == prev_devid && physical_offset < prev_dev_ext_end) { > + btrfs_err(fs_info, > +"dev extent devid %llu physical offset %llu overlap with previous dev extent end %llu", > + devid, physical_offset, prev_dev_ext_end); > + ret = -EUCLEAN; > + goto out; > + } > + > ret = verify_one_dev_extent(fs_info, chunk_offset, devid, > physical_offset, physical_len); > if (ret < 0) > goto out; > + prev_devid = devid; > + prev_dev_ext_end = physical_offset + physical_len; > + > ret = btrfs_next_item(root, path); > if (ret < 0) > goto out; >
On 2018/10/5 下午4:56, Nikolay Borisov wrote: > > > On 5.10.2018 11:25, Qu Wenruo wrote: >> Enhance btrfs_verify_dev_extents() to remember previous checked dev >> extents, so it can check if one dev extent overlaps with previous dev extent. >> >> Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > If this is related to the initial report here: > > https://lore.kernel.org/linux-btrfs/782f6000-30c0-0085-abd2-74ec5827c903@mendix.com/T/#m609ccb5d32998e8ba5cfa9901c1ab56a38a6f374 > > I'd rather have it as a link to the commit: > > Link: > https://lore.kernel.org/linux-btrfs/b3461a38-e5f8-f41d-c67c-2efac8129054@mendix.com/ I'd prefer something shorter, like. Link: https://www.spinics.net/lists/linux-btrfs/msg82370.html Is there any shorter URL version from lore.kernel.org? Will add such tag in next version. Thanks, Qu > > Since I wouldn't expect this occur but clearly there is a bug. > > >> --- >> fs/btrfs/volumes.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index da86706123ff..bf0b2c16847a 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -7462,6 +7462,8 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) >> struct btrfs_path *path; >> struct btrfs_root *root = fs_info->dev_root; >> struct btrfs_key key; >> + u64 prev_devid = 0; >> + u64 prev_dev_ext_end = 0; >> int ret = 0; >> >> key.objectid = 1; >> @@ -7506,10 +7508,22 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) >> chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dext); >> physical_len = btrfs_dev_extent_length(leaf, dext); >> >> + /* Check if this dev extent over lap with previous one */ >> + if (devid == prev_devid && physical_offset < prev_dev_ext_end) { >> + btrfs_err(fs_info, >> +"dev extent devid %llu physical offset %llu overlap with previous dev extent end %llu", >> + devid, physical_offset, prev_dev_ext_end); >> + ret = -EUCLEAN; >> + goto out; >> + } >> + >> ret = verify_one_dev_extent(fs_info, chunk_offset, devid, >> physical_offset, physical_len); >> if (ret < 0) >> goto out; >> + prev_devid = devid; >> + prev_dev_ext_end = physical_offset + physical_len; >> + >> ret = btrfs_next_item(root, path); >> if (ret < 0) >> goto out; >>
On Fri, Oct 05, 2018 at 05:22:40PM +0800, Qu Wenruo wrote: > > > On 2018/10/5 下午4:56, Nikolay Borisov wrote: > > > > > > On 5.10.2018 11:25, Qu Wenruo wrote: > >> Enhance btrfs_verify_dev_extents() to remember previous checked dev > >> extents, so it can check if one dev extent overlaps with previous dev extent. > >> > >> Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> > >> Signed-off-by: Qu Wenruo <wqu@suse.com> > > > > If this is related to the initial report here: > > > > https://lore.kernel.org/linux-btrfs/782f6000-30c0-0085-abd2-74ec5827c903@mendix.com/T/#m609ccb5d32998e8ba5cfa9901c1ab56a38a6f374 > > > > I'd rather have it as a link to the commit: > > > > Link: > > https://lore.kernel.org/linux-btrfs/b3461a38-e5f8-f41d-c67c-2efac8129054@mendix.com/ > > I'd prefer something shorter, like. > Link: https://www.spinics.net/lists/linux-btrfs/msg82370.html > > Is there any shorter URL version from lore.kernel.org? https://lkml.kernel.org/r/<message-id> or https://lore.kernel.org/linux-btrfs/<message-id>
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index da86706123ff..bf0b2c16847a 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -7462,6 +7462,8 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) struct btrfs_path *path; struct btrfs_root *root = fs_info->dev_root; struct btrfs_key key; + u64 prev_devid = 0; + u64 prev_dev_ext_end = 0; int ret = 0; key.objectid = 1; @@ -7506,10 +7508,22 @@ int btrfs_verify_dev_extents(struct btrfs_fs_info *fs_info) chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dext); physical_len = btrfs_dev_extent_length(leaf, dext); + /* Check if this dev extent over lap with previous one */ + if (devid == prev_devid && physical_offset < prev_dev_ext_end) { + btrfs_err(fs_info, +"dev extent devid %llu physical offset %llu overlap with previous dev extent end %llu", + devid, physical_offset, prev_dev_ext_end); + ret = -EUCLEAN; + goto out; + } + ret = verify_one_dev_extent(fs_info, chunk_offset, devid, physical_offset, physical_len); if (ret < 0) goto out; + prev_devid = devid; + prev_dev_ext_end = physical_offset + physical_len; + ret = btrfs_next_item(root, path); if (ret < 0) goto out;
Enhance btrfs_verify_dev_extents() to remember previous checked dev extents, so it can check if one dev extent overlaps with previous dev extent. Reported-by: Hans van Kranenburg <hans.van.kranenburg@mendix.com> Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/volumes.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)