Skip to content

Commit 7e06de7

Browse files
adam900710kdave
authored andcommitted
btrfs: canonicalize the device path before adding it
[PROBLEM] Currently btrfs accepts any file path for its device, resulting some weird situation: # ./mount_by_fd /dev/test/scratch1 /mnt/btrfs/ The program has the following source code: #include <fcntl.h> #include <stdio.h> #include <sys/mount.h> int main(int argc, char *argv[]) { int fd = open(argv[1], O_RDWR); char path[256]; snprintf(path, sizeof(path), "/proc/self/fd/%d", fd); return mount(path, argv[2], "btrfs", 0, NULL); } Then we can have the following weird device path: BTRFS: device fsid 2378be81-fe12-46d2-a9e8-68cf08dd98d5 devid 1 transid 7 /proc/self/fd/3 (253:2) scanned by mount_by_fd (18440) Normally it's not a big deal, and later udev can trigger a device path rename. But if udev didn't trigger, the device path "/proc/self/fd/3" will show up in mtab. [CAUSE] For filename "/proc/self/fd/3", it means the opened file descriptor 3. In above case, it's exactly the device we want to open, aka points to "/dev/test/scratch1" which is another symlink pointing to "/dev/dm-2". Inside kernel we solve the mount source using LOOKUP_FOLLOW, which follows the symbolic link and grab the proper block device. But inside btrfs we also save the filename into btrfs_device::name, and utilize that member to report our mount source, which leads to the above situation. [FIX] Instead of unconditionally trust the path, check if the original file (not following the symbolic link) is inside "/dev/", if not, then manually lookup the path to its final destination, and use that as our device path. This allows us to still use symbolic links, like "/dev/mapper/test-scratch" from LVM2, which is required for fstests runs with LVM2 setup. And for really weird names, like the above case, we solve it to "/dev/dm-2" instead. Reviewed-by: Filipe Manana <[email protected]> Link: https://bugzilla.suse.com/show_bug.cgi?id=1230641 Reported-by: Fabian Vogt <[email protected]> Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 2e8b6bc commit 7e06de7

File tree

1 file changed

+86
-1
lines changed

1 file changed

+86
-1
lines changed

fs/btrfs/volumes.c

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,78 @@ const u8 *btrfs_sb_fsid_ptr(const struct btrfs_super_block *sb)
732732
return has_metadata_uuid ? sb->metadata_uuid : sb->fsid;
733733
}
734734

735+
/*
736+
* We can have very weird soft links passed in.
737+
* One example is "/proc/self/fd/<fd>", which can be a soft link to
738+
* a block device.
739+
*
740+
* But it's never a good idea to use those weird names.
741+
* Here we check if the path (not following symlinks) is a good one inside
742+
* "/dev/".
743+
*/
744+
static bool is_good_dev_path(const char *dev_path)
745+
{
746+
struct path path = { .mnt = NULL, .dentry = NULL };
747+
char *path_buf = NULL;
748+
char *resolved_path;
749+
bool is_good = false;
750+
int ret;
751+
752+
if (!dev_path)
753+
goto out;
754+
755+
path_buf = kmalloc(PATH_MAX, GFP_KERNEL);
756+
if (!path_buf)
757+
goto out;
758+
759+
/*
760+
* Do not follow soft link, just check if the original path is inside
761+
* "/dev/".
762+
*/
763+
ret = kern_path(dev_path, 0, &path);
764+
if (ret)
765+
goto out;
766+
resolved_path = d_path(&path, path_buf, PATH_MAX);
767+
if (IS_ERR(resolved_path))
768+
goto out;
769+
if (strncmp(resolved_path, "/dev/", strlen("/dev/")))
770+
goto out;
771+
is_good = true;
772+
out:
773+
kfree(path_buf);
774+
path_put(&path);
775+
return is_good;
776+
}
777+
778+
static int get_canonical_dev_path(const char *dev_path, char *canonical)
779+
{
780+
struct path path = { .mnt = NULL, .dentry = NULL };
781+
char *path_buf = NULL;
782+
char *resolved_path;
783+
int ret;
784+
785+
if (!dev_path) {
786+
ret = -EINVAL;
787+
goto out;
788+
}
789+
790+
path_buf = kmalloc(PATH_MAX, GFP_KERNEL);
791+
if (!path_buf) {
792+
ret = -ENOMEM;
793+
goto out;
794+
}
795+
796+
ret = kern_path(dev_path, LOOKUP_FOLLOW, &path);
797+
if (ret)
798+
goto out;
799+
resolved_path = d_path(&path, path_buf, PATH_MAX);
800+
ret = strscpy(canonical, resolved_path, PATH_MAX);
801+
out:
802+
kfree(path_buf);
803+
path_put(&path);
804+
return ret;
805+
}
806+
735807
static bool is_same_device(struct btrfs_device *device, const char *new_path)
736808
{
737809
struct path old = { .mnt = NULL, .dentry = NULL };
@@ -1419,12 +1491,23 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
14191491
bool new_device_added = false;
14201492
struct btrfs_device *device = NULL;
14211493
struct file *bdev_file;
1494+
char *canonical_path = NULL;
14221495
u64 bytenr;
14231496
dev_t devt;
14241497
int ret;
14251498

14261499
lockdep_assert_held(&uuid_mutex);
14271500

1501+
if (!is_good_dev_path(path)) {
1502+
canonical_path = kmalloc(PATH_MAX, GFP_KERNEL);
1503+
if (canonical_path) {
1504+
ret = get_canonical_dev_path(path, canonical_path);
1505+
if (ret < 0) {
1506+
kfree(canonical_path);
1507+
canonical_path = NULL;
1508+
}
1509+
}
1510+
}
14281511
/*
14291512
* Avoid an exclusive open here, as the systemd-udev may initiate the
14301513
* device scan which may race with the user's mount or mkfs command,
@@ -1469,7 +1552,8 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
14691552
goto free_disk_super;
14701553
}
14711554

1472-
device = device_list_add(path, disk_super, &new_device_added);
1555+
device = device_list_add(canonical_path ? : path, disk_super,
1556+
&new_device_added);
14731557
if (!IS_ERR(device) && new_device_added)
14741558
btrfs_free_stale_devices(device->devt, device);
14751559

@@ -1478,6 +1562,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, blk_mode_t flags,
14781562

14791563
error_bdev_put:
14801564
fput(bdev_file);
1565+
kfree(canonical_path);
14811566

14821567
return device;
14831568
}

0 commit comments

Comments
 (0)