Skip to content

Commit 5ab2b18

Browse files
committed
btrfs: factor out validation of btrfs_ioctl_vol_args::name
The validation of vol args name in several 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 f33163e commit 5ab2b18

3 files changed

Lines changed: 35 additions & 6 deletions

File tree

fs/btrfs/fs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,6 +973,8 @@ void btrfs_exclop_finish(struct btrfs_fs_info *fs_info);
973973
void btrfs_exclop_balance(struct btrfs_fs_info *fs_info,
974974
enum btrfs_exclusive_operation op);
975975

976+
int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args);
977+
976978
/* Compatibility and incompatibility defines */
977979
void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag,
978980
const char *name);

fs/btrfs/ioctl.c

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,13 @@ static int check_fsflags_compatible(struct btrfs_fs_info *fs_info,
227227
return 0;
228228
}
229229

230+
int btrfs_check_ioctl_vol_args_path(const struct btrfs_ioctl_vol_args *vol_args)
231+
{
232+
if (memchr(vol_args->name, 0, sizeof(vol_args->name)) == NULL)
233+
return -ENAMETOOLONG;
234+
return 0;
235+
}
236+
230237
/*
231238
* Set flags/xflags from the internal inode flags. The remaining items of
232239
* fsxattr are zeroed.
@@ -1126,7 +1133,10 @@ static noinline int btrfs_ioctl_resize(struct file *file,
11261133
ret = PTR_ERR(vol_args);
11271134
goto out_drop;
11281135
}
1129-
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
1136+
ret = btrfs_check_ioctl_vol_args_path(vol_args);
1137+
if (ret < 0)
1138+
goto out_free;
1139+
11301140
sizestr = vol_args->name;
11311141
cancel = (strcmp("cancel", sizestr) == 0);
11321142
ret = exclop_start_or_cancel_reloc(fs_info, BTRFS_EXCLOP_RESIZE, cancel);
@@ -1326,12 +1336,15 @@ static noinline int btrfs_ioctl_snap_create(struct file *file,
13261336
vol_args = memdup_user(arg, sizeof(*vol_args));
13271337
if (IS_ERR(vol_args))
13281338
return PTR_ERR(vol_args);
1329-
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
1339+
ret = btrfs_check_ioctl_vol_args_path(vol_args);
1340+
if (ret < 0)
1341+
goto out;
13301342

13311343
ret = __btrfs_ioctl_snap_create(file, file_mnt_idmap(file),
13321344
vol_args->name, vol_args->fd, subvol,
13331345
false, NULL);
13341346

1347+
out:
13351348
kfree(vol_args);
13361349
return ret;
13371350
}
@@ -2464,7 +2477,10 @@ static noinline int btrfs_ioctl_snap_destroy(struct file *file,
24642477
if (IS_ERR(vol_args))
24652478
return PTR_ERR(vol_args);
24662479

2467-
vol_args->name[BTRFS_PATH_NAME_MAX] = 0;
2480+
err = btrfs_check_ioctl_vol_args_path(vol_args);
2481+
if (err < 0)
2482+
goto out;
2483+
24682484
subvol_name = vol_args->name;
24692485

24702486
err = mnt_want_write_file(file);
@@ -2675,12 +2691,16 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg)
26752691
goto out;
26762692
}
26772693

2678-
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
2694+
ret = btrfs_check_ioctl_vol_args_path(vol_args);
2695+
if (ret < 0)
2696+
goto out_free;
2697+
26792698
ret = btrfs_init_new_device(fs_info, vol_args->name);
26802699

26812700
if (!ret)
26822701
btrfs_info(fs_info, "disk added %s", vol_args->name);
26832702

2703+
out_free:
26842704
kfree(vol_args);
26852705
out:
26862706
if (restore_op)
@@ -2772,7 +2792,10 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
27722792
if (IS_ERR(vol_args))
27732793
return PTR_ERR(vol_args);
27742794

2775-
vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
2795+
ret = btrfs_check_ioctl_vol_args_path(vol_args);
2796+
if (ret < 0)
2797+
goto out_free;
2798+
27762799
if (!strcmp("cancel", vol_args->name)) {
27772800
cancel = true;
27782801
} else {
@@ -2799,6 +2822,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
27992822
bdev_release(bdev_handle);
28002823
out:
28012824
btrfs_put_dev_args_from_path(&args);
2825+
out_free:
28022826
kfree(vol_args);
28032827
return ret;
28042828
}

fs/btrfs/super.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2201,7 +2201,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
22012201
vol = memdup_user((void __user *)arg, sizeof(*vol));
22022202
if (IS_ERR(vol))
22032203
return PTR_ERR(vol);
2204-
vol->name[BTRFS_PATH_NAME_MAX] = '\0';
2204+
ret = btrfs_check_ioctl_vol_args_path(vol);
2205+
if (ret < 0)
2206+
goto out;
22052207

22062208
switch (cmd) {
22072209
case BTRFS_IOC_SCAN_DEV:
@@ -2243,6 +2245,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
22432245
break;
22442246
}
22452247

2248+
out:
22462249
kfree(vol);
22472250
return ret;
22482251
}

0 commit comments

Comments
 (0)