Re: [PATCH v2] fuse: allow synchronous FUSE_INIT

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Aug 27, 2025 at 4:00 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> FUSE_INIT has always been asynchronous with mount.  That means that the
> server processed this request after the mount syscall returned.
>
> This means that FUSE_INIT can't supply the root inode's ID, hence it
> currently has a hardcoded value.  There are other limitations such as not
> being able to perform getxattr during mount, which is needed by selinux.
>
> To remove these limitations allow server to process FUSE_INIT while
> initializing the in-core super block for the fuse filesystem.  This can
> only be done if the server is prepared to handle this, so add
> FUSE_DEV_IOC_SYNC_INIT ioctl, which
>
>  a) lets the server know whether this feature is supported, returning
>  ENOTTY othewrwise.
>
>  b) lets the kernel know to perform a synchronous initialization
>
> The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> only during super block creation.  This is solved by setting the private
> data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> waiting for this to be turned into a proper fuse_dev before commecing with
> operations on the device file.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> v2:
>
>  - make fuse_send_init() perform sync/async sequence based on fc->sync_init
>    (Joanne)
>
> fs/fuse/cuse.c            |  3 +-
>  fs/fuse/dev.c             | 74 +++++++++++++++++++++++++++++----------
>  fs/fuse/dev_uring.c       |  4 +--
>  fs/fuse/fuse_dev_i.h      | 13 +++++--
>  fs/fuse/fuse_i.h          |  5 ++-
>  fs/fuse/inode.c           | 50 ++++++++++++++++++++------
>  include/uapi/linux/fuse.h |  1 +
>  7 files changed, 115 insertions(+), 35 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 8ac074414897..948f45c6e0ef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1530,14 +1530,34 @@ static int fuse_dev_open(struct inode *inode, struct file *file)
>         return 0;
>  }
>
> +struct fuse_dev *fuse_get_dev(struct file *file)
> +{
> +       struct fuse_dev *fud = __fuse_get_dev(file);
> +       int err;
> +
> +       if (likely(fud))
> +               return fud;
> +
> +       err = wait_event_interruptible(fuse_dev_waitq,
> +                                      READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);

I wonder if we should make the semantics the same for synchronous and
non-synchronous inits here, i.e. doing a wait for
"(READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT) &&
READ_ONCE(file->private_data) != NULL", so that from the libfuse point
of view, the flow can be unified between the two, eg
i) send sync_init ioctl call if doing a synchronous init
ii) kick off thread to read requests
iii) do mount call
otherwise for async inits, the mount call needs to happen first.

> +       if (err)
> +               return ERR_PTR(err);
> +
> +       fud = __fuse_get_dev(file);
> +       if (!fud)
> +               return ERR_PTR(-EPERM);
> +
> +       return fud;
> +}
> +
>  static ssize_t fuse_dev_read(struct kiocb *iocb, struct iov_iter *to)
>  {
>         struct fuse_copy_state cs;
>         struct file *file = iocb->ki_filp;
>         struct fuse_dev *fud = fuse_get_dev(file);
>
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         if (!user_backed_iter(to))
>                 return -EINVAL;
> @@ -1557,8 +1577,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
>         struct fuse_copy_state cs;
>         struct fuse_dev *fud = fuse_get_dev(in);
>
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer),
>                               GFP_KERNEL);
> @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
>         struct fuse_copy_state cs;
>         struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);

Does this (and below in fuse_dev_splice_write()) need to be
fuse_get_dev()? afaict, fuse_dev_write() only starts getting used
after fud has already been initialized. i see why it's needed for
fuse_dev_read() since otherwise the server doesn't know when it can
start calling fuse_dev_read(), but for fuse_dev_write(), it seems like
that only gets used after fud is already initialized.

>
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         if (!user_backed_iter(from))
>                 return -EINVAL;
> @@ -2258,8 +2278,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
>         ssize_t ret;
>
>         fud = fuse_get_dev(out);
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         pipe_lock(pipe);
>
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
>         struct fuse_dev *fud = fuse_get_dev(file);

Should this be __fuse_get_dev()?

>         struct fuse_backing_map map;
>
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
>                 return -EOPNOTSUPP;
> @@ -2598,8 +2618,8 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
>         struct fuse_dev *fud = fuse_get_dev(file);

Same question here.

>         int backing_id;
>
> -       if (!fud)
> -               return -EPERM;
> +       if (IS_ERR(fud))
> +               return PTR_ERR(fud);
>
>         if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
>                 return -EOPNOTSUPP;
> @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
>         return fuse_backing_close(fud->fc, backing_id);
>  }
>
> +static long fuse_dev_ioctl_sync_init(struct file *file)
> +{
> +       int err = -EINVAL;
> +
> +       mutex_lock(&fuse_mutex);
> +       if (!__fuse_get_dev(file)) {
> +               WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);

Does this still need a WRITE_ONCE if it's accessed within the scope of
the mutex? My understanding (maybe wrong) is that a mutex implicitly
serves as also a memory barrier. If not, then we probably also need a
WRITE_ONCE() around the *ctx->fudptr assignment in
fuse_fill_super_common()?


Thanks,
Joanne

> +               err = 0;
> +       }
> +       mutex_unlock(&fuse_mutex);
> +       return err;
> +}
> +
> @@ -1876,8 +1901,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
>         list_add_tail(&fc->entry, &fuse_conn_list);
>         sb->s_root = root_dentry;
> -       if (ctx->fudptr)
> +       if (ctx->fudptr) {
>                 *ctx->fudptr = fud;
> +               wake_up_all(&fuse_dev_waitq);
> +       }
>         mutex_unlock(&fuse_mutex);
>         return 0;
>