| Message ID | 73979e98c7edc6690959f1d5e9e8d2bb678a8101.1661473186.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [STABLE,5.4] btrfs: harden identification of a stale device | expand |
On Fri, Sep 02, 2022 at 05:09:38PM +0800, Anand Jain wrote: > commit 770c79fb65506fc7c16459855c3839429f46cb32 upstream > > Identifying and removing the stale device from the fs_uuids list is done > by btrfs_free_stale_devices(). btrfs_free_stale_devices() in turn > depends on device_path_matched() to check if the device appears in more > than one btrfs_device structure. > > The matching of the device happens by its path, the device path. However, > when device mapper is in use, the dm device paths are nothing but a link > to the actual block device, which leads to the device_path_matched() > failing to match. > > Fix this by matching the dev_t as provided by lookup_bdev() instead of > plain string compare of the device paths. > > CC: stable@vger.kernel.org #5.4 > Reported-by: Josef Bacik <josef@toxicpanda.com> > Signed-off-by: Anand Jain <anand.jain@oracle.com> > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/volumes.c | 44 +++++++++++++++++++++++++++++++++++++------- > 1 file changed, 37 insertions(+), 7 deletions(-) What about the same change for 5.10.y? thanks, greg k-h
On 9/2/22 17:23, Greg KH wrote: > On Fri, Sep 02, 2022 at 05:09:38PM +0800, Anand Jain wrote: >> commit 770c79fb65506fc7c16459855c3839429f46cb32 upstream >> >> Identifying and removing the stale device from the fs_uuids list is done >> by btrfs_free_stale_devices(). btrfs_free_stale_devices() in turn >> depends on device_path_matched() to check if the device appears in more >> than one btrfs_device structure. >> >> The matching of the device happens by its path, the device path. However, >> when device mapper is in use, the dm device paths are nothing but a link >> to the actual block device, which leads to the device_path_matched() >> failing to match. >> >> Fix this by matching the dev_t as provided by lookup_bdev() instead of >> plain string compare of the device paths. >> >> CC: stable@vger.kernel.org #5.4 >> Reported-by: Josef Bacik <josef@toxicpanda.com> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> Signed-off-by: David Sterba <dsterba@suse.com> >> --- >> fs/btrfs/volumes.c | 44 +++++++++++++++++++++++++++++++++++++------- >> 1 file changed, 37 insertions(+), 7 deletions(-) > > What about the same change for 5.10.y? > Thanks for reminding me. I have sent a separate patch for 5.10 as this patch won't apply to 5.10. - Anand > thanks, > > greg k-h
On Sat, Sep 03, 2022 at 08:25:12PM +0800, Anand Jain wrote: > On 9/2/22 17:23, Greg KH wrote: > > On Fri, Sep 02, 2022 at 05:09:38PM +0800, Anand Jain wrote: > > > commit 770c79fb65506fc7c16459855c3839429f46cb32 upstream > > > > > > Identifying and removing the stale device from the fs_uuids list is done > > > by btrfs_free_stale_devices(). btrfs_free_stale_devices() in turn > > > depends on device_path_matched() to check if the device appears in more > > > than one btrfs_device structure. > > > > > > The matching of the device happens by its path, the device path. However, > > > when device mapper is in use, the dm device paths are nothing but a link > > > to the actual block device, which leads to the device_path_matched() > > > failing to match. > > > > > > Fix this by matching the dev_t as provided by lookup_bdev() instead of > > > plain string compare of the device paths. > > > > > > CC: stable@vger.kernel.org #5.4 > > > Reported-by: Josef Bacik <josef@toxicpanda.com> > > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > > > Signed-off-by: David Sterba <dsterba@suse.com> > > > --- > > > fs/btrfs/volumes.c | 44 +++++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > What about the same change for 5.10.y? > > > > Thanks for reminding me. > I have sent a separate patch for 5.10 as this patch won't apply to 5.10. Both now queued up, thanks. greg k-h
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index c7706a769de1..548de841cee5 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -713,15 +713,47 @@ static void pending_bios_fn(struct btrfs_work *work) run_scheduled_bios(device); } -static bool device_path_matched(const char *path, struct btrfs_device *device) +/* + * Check if the device in the path matches the device in the given struct device. + * + * Returns: + * true If it is the same device. + * false If it is not the same device or on error. + */ +static bool device_matched(const struct btrfs_device *device, const char *path) { - int found; + char *device_name; + struct block_device *bdev_old; + struct block_device *bdev_new; + + /* + * If we are looking for a device with the matching dev_t, then skip + * device without a name (a missing device). + */ + if (!device->name) + return false; + + device_name = kzalloc(BTRFS_PATH_NAME_MAX, GFP_KERNEL); + if (!device_name) + return false; rcu_read_lock(); - found = strcmp(rcu_str_deref(device->name), path); + scnprintf(device_name, BTRFS_PATH_NAME_MAX, "%s", rcu_str_deref(device->name)); rcu_read_unlock(); - return found == 0; + bdev_old = lookup_bdev(device_name); + kfree(device_name); + if (IS_ERR(bdev_old)) + return false; + + bdev_new = lookup_bdev(path); + if (IS_ERR(bdev_new)) + return false; + + if (bdev_old == bdev_new) + return true; + + return false; } /* @@ -754,9 +786,7 @@ static int btrfs_free_stale_devices(const char *path, &fs_devices->devices, dev_list) { if (skip_device && skip_device == device) continue; - if (path && !device->name) - continue; - if (path && !device_path_matched(path, device)) + if (path && !device_matched(device, path)) continue; if (fs_devices->opened) { /* for an already deleted device return 0 */