Skip to content

Commit 0478adf

Browse files
committed
btrfs: factor out validation of btrfs_ioctl_vol_args_v2::name
The validation of vol args v2 name in snapshot and device remove ioctls is not done properly. A terminating NUL is written to the end of the buffer unconditionally, assuming that this would be the last place in case the buffer is used completely. This does not communicate back the actual error (either an invalid or too long path). Factor out all such cases and use a helper to do the verification, simply look for NUL in the buffer. There's no expected practical change, the size of buffer is 4088, this is enough for most paths or names. Reviewed-by: Boris Burkov <boris@bur.io> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 5ab2b18 commit 0478adf

1 file changed

Lines changed: 17 additions & 3 deletions

File tree

fs/btrfs/ioctl.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,13 @@ int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args)
234234
return 0;
235235
}
236236

237+
static int btrfs_check_ioctl_vol_args2_subvol_name(const struct btrfs_ioctl_vol_args_v2 *vol_args2)
238+
{
239+
if (memchr(vol_args2->name, 0, sizeof(vol_args2->name)) == NULL)
240+
return -ENAMETOOLONG;
241+
return 0;
242+
}
243+
237244
/*
238245
* Set flags/xflags from the internal inode flags. The remaining items of
239246
* fsxattr are zeroed.
@@ -1363,7 +1370,9 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
13631370
vol_args = memdup_user(arg, sizeof(*vol_args));
13641371
if (IS_ERR(vol_args))
13651372
return PTR_ERR(vol_args);
1366-
vol_args->name[BTRFS_SUBVOL_NAME_MAX] = '\0';
1373+
ret = btrfs_check_ioctl_vol_args2_subvol_name(vol_args);
1374+
if (ret < 0)
1375+
goto free_args;
13671376

13681377
if (vol_args->flags & ~BTRFS_SUBVOL_CREATE_ARGS_MASK) {
13691378
ret = -EOPNOTSUPP;
@@ -2393,7 +2402,9 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
23932402
* name, same as v1 currently does.
23942403
*/
23952404
if (!(vol_args2->flags & BTRFS_SUBVOL_SPEC_BY_ID)) {
2396-
vol_args2->name[BTRFS_SUBVOL_NAME_MAX] = 0;
2405+
err = btrfs_check_ioctl_vol_args2_subvol_name(vol_args2);
2406+
if (err < 0)
2407+
goto out;
23972408
subvol_name = vol_args2->name;
23982409

23992410
err = mnt_want_write_file(file);
@@ -2732,7 +2743,10 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
27322743
goto out;
27332744
}
27342745

2735-
vol_args->name[BTRFS_SUBVOL_NAME_MAX] = '\0';
2746+
ret = btrfs_check_ioctl_vol_args2_subvol_name(vol_args);
2747+
if (ret < 0)
2748+
goto out;
2749+
27362750
if (vol_args->flags & BTRFS_DEVICE_SPEC_BY_ID) {
27372751
args.devid = vol_args->devid;
27382752
} else if (!strcmp("cancel", vol_args->name)) {

0 commit comments

Comments
 (0)