On Wed, Jan 10, 2024 at 12:55 AM David Sterba dsterba@suse.cz wrote:
On Wed, Jan 10, 2024 at 08:58:26AM +1030, Qu Wenruo wrote:
Add extra sanity check for btrfs_ioctl_defrag_range_args::flags.
This is not really to enhance fuzzing tests, but as a preparation for future expansion on btrfs_ioctl_defrag_range_args.
In the future we're adding new members, allowing more fine tuning for btrfs defrag. Without the -ENONOTSUPP error, there would be no way to detect if the kernel supports those new defrag features.
cc: stable@vger.kernel.org #4.14+ Signed-off-by: Qu Wenruo wqu@suse.com
Added to misc-next, thanks.
fs/btrfs/ioctl.c | 4 ++++ include/uapi/linux/btrfs.h | 2 ++ 2 files changed, 6 insertions(+)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index a1743904202b..3a846b983b28 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2608,6 +2608,10 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp) ret = -EFAULT; goto out; }
if (range.flags & ~BTRFS_DEFRAG_RANGE_FLAGS_SUPP) {ret = -EOPNOTSUPP;This should be EINVAL, this is for invalid parameter values or combinations, EOPNOTSUPP would be for the whole ioctl as not supported.
I'm confused now. We return EOPNOTSUPP for a lot of ioctls when they are given an unknown flag, for example at btrfs_ioctl_scrub():
if (sa->flags & ~BTRFS_SCRUB_SUPPORTED_FLAGS) { ret = -EOPNOTSUPP; goto out; }
Or at btrfs_ioctl_snap_create_v2():
if (vol_args->flags & ~BTRFS_SUBVOL_CREATE_ARGS_MASK) { ret = -EOPNOTSUPP; goto free_args; }
We also do similar for fallocate, at btrfs_fallocate():
if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE)) return -EOPNOTSUPP;
I was under the expectation that EOPNOTSUPP is the correct thing to do in this patch. So what's different in this patch from those existing examples to justify EINVAL instead?
Thanks.