Re: [PATCH RESEND V12 2/8] fuse: 32-bit user space ioctl compat for fuse device

From: Alessio Balsini
Date: Mon Mar 01 2021 - 07:28:03 EST


On Wed, Feb 17, 2021 at 11:21:04AM +0100, Miklos Szeredi wrote:
> On Mon, Jan 25, 2021 at 4:31 PM Alessio Balsini <balsini@xxxxxxxxxxx> wrote:
> >
> > With a 64-bit kernel build the FUSE device cannot handle ioctl requests
> > coming from 32-bit user space.
> > This is due to the ioctl command translation that generates different
> > command identifiers that thus cannot be used for direct comparisons
> > without proper manipulation.
> >
> > Explicitly extract type and number from the ioctl command to enable
> > 32-bit user space compatibility on 64-bit kernel builds.
> >
> > Signed-off-by: Alessio Balsini <balsini@xxxxxxxxxxx>
> > ---
> > fs/fuse/dev.c | 29 ++++++++++++++++++-----------
> > include/uapi/linux/fuse.h | 3 ++-
> > 2 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 588f8d1240aa..ff9f3b83f879 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -2233,37 +2233,44 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> > static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> > unsigned long arg)
> > {
> > - int err = -ENOTTY;
> > + int res;
> > + int oldfd;
> > + struct fuse_dev *fud = NULL;
> >
> > - if (cmd == FUSE_DEV_IOC_CLONE) {
> > - int oldfd;
> > + if (_IOC_TYPE(cmd) != FUSE_DEV_IOC_MAGIC)
> > + return -EINVAL;
>
> Why change ENOTTY to EINVAL?
>
> Thanks,
> MIklos

For the magic number mismatch I was thinking that EINVAL would have been
more appropriate, meaning: "this ioctl is definitely something this file
doesn't support". ENOTTY, could be more specific to the subset of
ioctls supported by the file, so I kept this in the default case of the
switch.
After counting the use of ENOTTY vs EINVAL for these _IOC_TYPE checks,
EINVALs were less than half ENOTTYs, so I'm totally fine with switching
back to ENOTTY in V13.

Thanks!
Alessio