* [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent @ 2019-01-08 6:08 Qu Wenruo 2019-01-08 11:16 ` Filipe Manana ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Qu Wenruo @ 2019-01-08 6:08 UTC (permalink / raw) To: linux-btrfs; +Cc: Filipe Manana [BUG] Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel message: BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 BTRFS error (device dm-6): open_ctree failed [CAUSE] Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") introduced strict check on dev extents. We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and only dependent on @devid to find the real device. For seed devices, we call clone_fs_devices() in open_seed_devices() to allow us search seed devices directly. However clone_fs_devices() just populates devices with devid and dev uuid, without populating other essential members, like disk_total_bytes. This makes any device returned by btrfs_find_device(fs_info, devid, NULL, NULL) is just a dummy, with 0 disk_total_bytes, and any dev extents on the seed device will not pass the device boundary check. [FIX] This patch will try to verify the device returned by btrfs_find_device() and if it's a dummy then re-search in seed devices. Reported-by: Filipe Manana <fdmanana@suse.com> Fixes: cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/volumes.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 2576b1a379c9..3e4f8f88353e 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -7825,6 +7825,18 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, ret = -EUCLEAN; goto out; } + + /* It's possible this device is a dummy for seed device */ + if (dev->disk_total_bytes == 0) { + dev = find_device(fs_info->fs_devices->seed, devid, NULL); + if (!dev) { + btrfs_err(fs_info, "failed to find seed devid %llu", + devid); + ret = -EUCLEAN; + goto out; + } + } + if (physical_offset + physical_len > dev->disk_total_bytes) { btrfs_err(fs_info, "dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu", -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent 2019-01-08 6:08 [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent Qu Wenruo @ 2019-01-08 11:16 ` Filipe Manana 2019-01-08 11:45 ` Qu Wenruo 2019-01-10 4:50 ` Qu Wenruo 2019-01-10 16:08 ` David Sterba 2 siblings, 1 reply; 6+ messages in thread From: Filipe Manana @ 2019-01-08 11:16 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, Filipe Manana On Tue, Jan 8, 2019 at 6:11 AM Qu Wenruo <wqu@suse.com> wrote: > > [BUG] > Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel > message: > > BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 > BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 > BTRFS error (device dm-6): open_ctree failed > > [CAUSE] > Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent > mapping check") introduced strict check on dev extents. > > We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and > only dependent on @devid to find the real device. > > For seed devices, we call clone_fs_devices() in open_seed_devices() to > allow us search seed devices directly. > > However clone_fs_devices() just populates devices with devid and dev > uuid, without populating other essential members, like disk_total_bytes. > > This makes any device returned by btrfs_find_device(fs_info, devid, > NULL, NULL) is just a dummy, with 0 disk_total_bytes, and any dev > extents on the seed device will not pass the device boundary check. > > [FIX] > This patch will try to verify the device returned by btrfs_find_device() > and if it's a dummy then re-search in seed devices. > > Reported-by: Filipe Manana <fdmanana@suse.com> > Fixes: cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/volumes.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 2576b1a379c9..3e4f8f88353e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -7825,6 +7825,18 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, > ret = -EUCLEAN; > goto out; > } > + > + /* It's possible this device is a dummy for seed device */ > + if (dev->disk_total_bytes == 0) { > + dev = find_device(fs_info->fs_devices->seed, devid, NULL); > + if (!dev) { > + btrfs_err(fs_info, "failed to find seed devid %llu", > + devid); > + ret = -EUCLEAN; > + goto out; > + } > + } Why just not pass the FSID (can be taken from fs_info->super_copy) to the previous call to btrfs_find_device()? It's a lot simpler. > + > if (physical_offset + physical_len > dev->disk_total_bytes) { > btrfs_err(fs_info, > "dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu", > -- > 2.20.1 > -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.” ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent 2019-01-08 11:16 ` Filipe Manana @ 2019-01-08 11:45 ` Qu Wenruo 2019-01-08 13:19 ` Qu Wenruo 0 siblings, 1 reply; 6+ messages in thread From: Qu Wenruo @ 2019-01-08 11:45 UTC (permalink / raw) To: fdmanana, Qu Wenruo; +Cc: linux-btrfs, Filipe Manana [-- Attachment #1.1: Type: text/plain, Size: 3138 bytes --] On 2019/1/8 下午7:16, Filipe Manana wrote: > On Tue, Jan 8, 2019 at 6:11 AM Qu Wenruo <wqu@suse.com> wrote: >> >> [BUG] >> Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel >> message: >> >> BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 >> BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 >> BTRFS error (device dm-6): open_ctree failed >> >> [CAUSE] >> Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent >> mapping check") introduced strict check on dev extents. >> >> We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and >> only dependent on @devid to find the real device. >> >> For seed devices, we call clone_fs_devices() in open_seed_devices() to >> allow us search seed devices directly. >> >> However clone_fs_devices() just populates devices with devid and dev >> uuid, without populating other essential members, like disk_total_bytes. >> >> This makes any device returned by btrfs_find_device(fs_info, devid, >> NULL, NULL) is just a dummy, with 0 disk_total_bytes, and any dev >> extents on the seed device will not pass the device boundary check. >> >> [FIX] >> This patch will try to verify the device returned by btrfs_find_device() >> and if it's a dummy then re-search in seed devices. >> >> Reported-by: Filipe Manana <fdmanana@suse.com> >> Fixes: cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/volumes.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >> index 2576b1a379c9..3e4f8f88353e 100644 >> --- a/fs/btrfs/volumes.c >> +++ b/fs/btrfs/volumes.c >> @@ -7825,6 +7825,18 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, >> ret = -EUCLEAN; >> goto out; >> } >> + >> + /* It's possible this device is a dummy for seed device */ >> + if (dev->disk_total_bytes == 0) { >> + dev = find_device(fs_info->fs_devices->seed, devid, NULL); >> + if (!dev) { >> + btrfs_err(fs_info, "failed to find seed devid %llu", >> + devid); >> + ret = -EUCLEAN; >> + goto out; >> + } >> + } > > Why just not pass the FSID (can be taken from fs_info->super_copy) to > the previous call to btrfs_find_device()? > It's a lot simpler. Then btrfs_find_device() will just return NULL. We still need to verify the dev extent of the seed device not to exceed seed device boundary. What we need is device uuid, however we can't easily get that dev uuid from dev extent item. Thanks, Qu > >> + >> if (physical_offset + physical_len > dev->disk_total_bytes) { >> btrfs_err(fs_info, >> "dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu", >> -- >> 2.20.1 >> > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent 2019-01-08 11:45 ` Qu Wenruo @ 2019-01-08 13:19 ` Qu Wenruo 0 siblings, 0 replies; 6+ messages in thread From: Qu Wenruo @ 2019-01-08 13:19 UTC (permalink / raw) To: fdmanana, Qu Wenruo; +Cc: linux-btrfs, Filipe Manana [-- Attachment #1.1: Type: text/plain, Size: 4465 bytes --] On 2019/1/8 下午7:45, Qu Wenruo wrote: > > > On 2019/1/8 下午7:16, Filipe Manana wrote: >> On Tue, Jan 8, 2019 at 6:11 AM Qu Wenruo <wqu@suse.com> wrote: >>> >>> [BUG] >>> Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel >>> message: >>> >>> BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 >>> BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 >>> BTRFS error (device dm-6): open_ctree failed >>> >>> [CAUSE] >>> Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent >>> mapping check") introduced strict check on dev extents. >>> >>> We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and >>> only dependent on @devid to find the real device. >>> >>> For seed devices, we call clone_fs_devices() in open_seed_devices() to >>> allow us search seed devices directly. >>> >>> However clone_fs_devices() just populates devices with devid and dev >>> uuid, without populating other essential members, like disk_total_bytes. >>> >>> This makes any device returned by btrfs_find_device(fs_info, devid, >>> NULL, NULL) is just a dummy, with 0 disk_total_bytes, and any dev >>> extents on the seed device will not pass the device boundary check. >>> >>> [FIX] >>> This patch will try to verify the device returned by btrfs_find_device() >>> and if it's a dummy then re-search in seed devices. >>> >>> Reported-by: Filipe Manana <fdmanana@suse.com> >>> Fixes: cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> fs/btrfs/volumes.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c >>> index 2576b1a379c9..3e4f8f88353e 100644 >>> --- a/fs/btrfs/volumes.c >>> +++ b/fs/btrfs/volumes.c >>> @@ -7825,6 +7825,18 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, >>> ret = -EUCLEAN; >>> goto out; >>> } >>> + >>> + /* It's possible this device is a dummy for seed device */ >>> + if (dev->disk_total_bytes == 0) { >>> + dev = find_device(fs_info->fs_devices->seed, devid, NULL); >>> + if (!dev) { >>> + btrfs_err(fs_info, "failed to find seed devid %llu", >>> + devid); >>> + ret = -EUCLEAN; >>> + goto out; >>> + } >>> + } >> >> Why just not pass the FSID (can be taken from fs_info->super_copy) to >> the previous call to btrfs_find_device()? >> It's a lot simpler. > > Then btrfs_find_device() will just return NULL. > We still need to verify the dev extent of the seed device not to exceed > seed device boundary. OK, it's not the case. Even we can pass correct dev uuid into btrfs_find_device(), it will still give us the dummy device, with 0 disk_total_bytes. The problem is, we have a screwed up fs_info->fs_devices for seed device from the very beginning. For a seeding fs, devid 1 is seed device and devid 2 is real rw device. Then our fs_devices looks like: fs_info->fs_devices |- devices | |- devid 1 | | dummy, devid 1 dev uuid, any else is unpopulated | | created by open_seed_devices()-> clone_fs_devices() | |- devid 2 | real one, devid 2 dev uuid, everything is good |- seeding |- devices |- devid 1 real seed device. So, at btrfs_find_device() call time, we must pass the *seeding* fsid, to locate the real seed device. Or we can only get a dummy. This patch is just a quick and dirty fix. The correct way to fix is to not screw up fs_devices any more with such meaningless dummy in fs_devices. Especially when btrfs_find_device() has already handled such case. I don't understand why we do such meaningless clone_fs_devices() call, but I don't think the proper fix may catch the v5.0 window. Thanks, Qu > > What we need is device uuid, however we can't easily get that dev uuid > from dev extent item. > > Thanks, > Qu > >> >>> + >>> if (physical_offset + physical_len > dev->disk_total_bytes) { >>> btrfs_err(fs_info, >>> "dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu", >>> -- >>> 2.20.1 >>> >> >> > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent 2019-01-08 6:08 [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent Qu Wenruo 2019-01-08 11:16 ` Filipe Manana @ 2019-01-10 4:50 ` Qu Wenruo 2019-01-10 16:08 ` David Sterba 2 siblings, 0 replies; 6+ messages in thread From: Qu Wenruo @ 2019-01-10 4:50 UTC (permalink / raw) To: Qu Wenruo, linux-btrfs; +Cc: Filipe Manana [-- Attachment #1.1: Type: text/plain, Size: 2515 bytes --] Gentle ping. Although this is just a dirty fix, it would be much better to avoid seed device users yelling at me. Thanks, Qu On 2019/1/8 下午2:08, Qu Wenruo wrote: > [BUG] > Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel > message: > > BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 > BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 > BTRFS error (device dm-6): open_ctree failed > > [CAUSE] > Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent > mapping check") introduced strict check on dev extents. > > We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and > only dependent on @devid to find the real device. > > For seed devices, we call clone_fs_devices() in open_seed_devices() to > allow us search seed devices directly. > > However clone_fs_devices() just populates devices with devid and dev > uuid, without populating other essential members, like disk_total_bytes. > > This makes any device returned by btrfs_find_device(fs_info, devid, > NULL, NULL) is just a dummy, with 0 disk_total_bytes, and any dev > extents on the seed device will not pass the device boundary check. > > [FIX] > This patch will try to verify the device returned by btrfs_find_device() > and if it's a dummy then re-search in seed devices. > > Reported-by: Filipe Manana <fdmanana@suse.com> > Fixes: cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent mapping check") > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/volumes.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 2576b1a379c9..3e4f8f88353e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -7825,6 +7825,18 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, > ret = -EUCLEAN; > goto out; > } > + > + /* It's possible this device is a dummy for seed device */ > + if (dev->disk_total_bytes == 0) { > + dev = find_device(fs_info->fs_devices->seed, devid, NULL); > + if (!dev) { > + btrfs_err(fs_info, "failed to find seed devid %llu", > + devid); > + ret = -EUCLEAN; > + goto out; > + } > + } > + > if (physical_offset + physical_len > dev->disk_total_bytes) { > btrfs_err(fs_info, > "dev extent devid %llu physical offset %llu len %llu is beyond device boundary %llu", > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent 2019-01-08 6:08 [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent Qu Wenruo 2019-01-08 11:16 ` Filipe Manana 2019-01-10 4:50 ` Qu Wenruo @ 2019-01-10 16:08 ` David Sterba 2 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2019-01-10 16:08 UTC (permalink / raw) To: Qu Wenruo; +Cc: linux-btrfs, Filipe Manana On Tue, Jan 08, 2019 at 02:08:18PM +0800, Qu Wenruo wrote: > [BUG] > Linux v5.0-rc1 will fail fstests/btrfs/163 with the following kernel > message: > > BTRFS error (device dm-6): dev extent devid 1 physical offset 13631488 len 8388608 is beyond device boundary 0 > BTRFS error (device dm-6): failed to verify dev extents against chunks: -117 > BTRFS error (device dm-6): open_ctree failed > > [CAUSE] > Commit cf90d884b347 ("btrfs: Introduce mount time chunk <-> dev extent > mapping check") introduced strict check on dev extents. > > We use btrfs_find_device() with dev uuid and fs uuid set to NULL, and > only dependent on @devid to find the real device. > > For seed devices, we call clone_fs_devices() in open_seed_devices() to > allow us search seed devices directly. > > However clone_fs_devices() just populates devices with devid and dev > uuid, without populating other essential members, like disk_total_bytes. That's quite fragile but works as a quick fix, queued for 5.0-rc, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-10 16:08 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-08 6:08 [For 5.0-rc PATCH] btrfs: Use real device structure to verify dev extent Qu Wenruo 2019-01-08 11:16 ` Filipe Manana 2019-01-08 11:45 ` Qu Wenruo 2019-01-08 13:19 ` Qu Wenruo 2019-01-10 4:50 ` Qu Wenruo 2019-01-10 16:08 ` David Sterba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).