On Wed, Jan 10, 2024 at 12:55 AM David Sterba <dsterba@xxxxxxx> 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@xxxxxxxxxxxxxxx #4.14+
> > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>
> 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.
>
