[PATCH] fuse: allow synchronous FUSE_INIT

[PATCH] fuse: allow synchronous FUSE_INIT

[Miklos Szeredi]

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>
---
I tested this with my raw-interface tester, so no libfuse update yet.  Will
work on that next.

 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          |  3 ++
 fs/fuse/inode.c           | 46 +++++++++++++++++++-----
 include/uapi/linux/fuse.h |  1 +
 7 files changed, 112 insertions(+), 32 deletions(-)

diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index b39844d75a80..28c96961e85d 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -52,6 +52,7 @@
 #include <linux/user_namespace.h>
 
 #include "fuse_i.h"
+#include "fuse_dev_i.h"
 
 #define CUSE_CONNTBL_LEN	64
 
@@ -547,7 +548,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
  */
 static int cuse_channel_release(struct inode *inode, struct file *file)
 {
-	struct fuse_dev *fud = file->private_data;
+	struct fuse_dev *fud = __fuse_get_dev(file);
 	struct cuse_conn *cc = fc_to_cc(fud->fc);
 
 	/* remove from the conntbl, no more access from this point on */
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);
+	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);
 
-	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);
 
@@ -2343,7 +2363,7 @@ static __poll_t fuse_dev_poll(struct file *file, poll_table *wait)
 	struct fuse_iqueue *fiq;
 	struct fuse_dev *fud = fuse_get_dev(file);
 
-	if (!fud)
+	if (IS_ERR(fud))
 		return EPOLLERR;
 
 	fiq = &fud->fc->iq;
@@ -2490,7 +2510,7 @@ void fuse_wait_aborted(struct fuse_conn *fc)
 
 int fuse_dev_release(struct inode *inode, struct file *file)
 {
-	struct fuse_dev *fud = fuse_get_dev(file);
+	struct fuse_dev *fud = __fuse_get_dev(file);
 
 	if (fud) {
 		struct fuse_conn *fc = fud->fc;
@@ -2521,8 +2541,8 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
 {
 	struct fuse_dev *fud = fuse_get_dev(file);
 
-	if (!fud)
-		return -EPERM;
+	if (IS_ERR(fud))
+		return PTR_ERR(fud);
 
 	/* No locking - fasync_helper does its own locking */
 	return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
@@ -2532,7 +2552,7 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
 {
 	struct fuse_dev *fud;
 
-	if (new->private_data)
+	if (__fuse_get_dev(new))
 		return -EINVAL;
 
 	fud = fuse_dev_alloc_install(fc);
@@ -2563,7 +2583,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
 	 * uses the same ioctl handler.
 	 */
 	if (fd_file(f)->f_op == file->f_op)
-		fud = fuse_get_dev(fd_file(f));
+		fud = __fuse_get_dev(fd_file(f));
 
 	res = -EINVAL;
 	if (fud) {
@@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
 	struct fuse_dev *fud = fuse_get_dev(file);
 	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);
 	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);
+		err = 0;
+	}
+	mutex_unlock(&fuse_mutex);
+	return err;
+}
+
 static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 			   unsigned long arg)
 {
@@ -2625,6 +2658,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 	case FUSE_DEV_IOC_BACKING_CLOSE:
 		return fuse_dev_ioctl_backing_close(file, argp);
 
+	case FUSE_DEV_IOC_SYNC_INIT:
+		return fuse_dev_ioctl_sync_init(file);
+
 	default:
 		return -ENOTTY;
 	}
@@ -2633,7 +2669,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 #ifdef CONFIG_PROC_FS
 static void fuse_dev_show_fdinfo(struct seq_file *seq, struct file *file)
 {
-	struct fuse_dev *fud = fuse_get_dev(file);
+	struct fuse_dev *fud = __fuse_get_dev(file);
 	if (!fud)
 		return;
 
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 249b210becb1..bef38ed78249 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -1139,9 +1139,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
 		return -EINVAL;
 
 	fud = fuse_get_dev(cmd->file);
-	if (!fud) {
+	if (IS_ERR(fud)) {
 		pr_info_ratelimited("No fuse device found\n");
-		return -ENOTCONN;
+		return PTR_ERR(fud);
 	}
 	fc = fud->fc;
 
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 5a9bd771a319..6e8373f97040 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -12,6 +12,8 @@
 #define FUSE_INT_REQ_BIT (1ULL << 0)
 #define FUSE_REQ_ID_STEP (1ULL << 1)
 
+extern struct wait_queue_head fuse_dev_waitq;
+
 struct fuse_arg;
 struct fuse_args;
 struct fuse_pqueue;
@@ -37,15 +39,22 @@ struct fuse_copy_state {
 	} ring;
 };
 
-static inline struct fuse_dev *fuse_get_dev(struct file *file)
+#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
+#define FUSE_DEV_PTR_MASK (~1UL)
+
+static inline struct fuse_dev *__fuse_get_dev(struct file *file)
 {
 	/*
 	 * Lockless access is OK, because file->private data is set
 	 * once during mount and is valid until the file is released.
 	 */
-	return READ_ONCE(file->private_data);
+	struct fuse_dev *fud = READ_ONCE(file->private_data);
+
+	return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
 }
 
+struct fuse_dev *fuse_get_dev(struct file *file);
+
 unsigned int fuse_req_hash(u64 unique);
 struct fuse_req *fuse_request_find(struct fuse_pqueue *fpq, u64 unique);
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 486fa550c951..54121207cfb2 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -904,6 +904,9 @@ struct fuse_conn {
 	/* Is link not implemented by fs? */
 	unsigned int no_link:1;
 
+	/* Is synchronous FUSE_INIT allowed? */
+	unsigned int sync_init:1;
+
 	/* Use io_uring for communication */
 	unsigned int io_uring;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 9d26a5bc394d..d5f9f2abc569 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -7,6 +7,7 @@
 */
 
 #include "fuse_i.h"
+#include "fuse_dev_i.h"
 #include "dev_uring_i.h"
 
 #include <linux/dax.h>
@@ -34,6 +35,7 @@ MODULE_LICENSE("GPL");
 static struct kmem_cache *fuse_inode_cachep;
 struct list_head fuse_conn_list;
 DEFINE_MUTEX(fuse_mutex);
+DECLARE_WAIT_QUEUE_HEAD(fuse_dev_waitq);
 
 static int set_global_limit(const char *val, const struct kernel_param *kp);
 
@@ -1466,7 +1468,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 	wake_up_all(&fc->blocked_waitq);
 }
 
-void fuse_send_init(struct fuse_mount *fm)
+static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
 {
 	struct fuse_init_args *ia;
 	u64 flags;
@@ -1525,8 +1527,15 @@ void fuse_send_init(struct fuse_mount *fm)
 	ia->args.out_args[0].value = &ia->out;
 	ia->args.force = true;
 	ia->args.nocreds = true;
-	ia->args.end = process_init_reply;
 
+	return ia;
+}
+
+void fuse_send_init(struct fuse_mount *fm)
+{
+	struct fuse_init_args *ia = fuse_new_init(fm);
+
+	ia->args.end = process_init_reply;
 	if (fuse_simple_background(fm, &ia->args, GFP_KERNEL) != 0)
 		process_init_reply(fm, &ia->args, -ENOTCONN);
 }
@@ -1867,8 +1876,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 
 	mutex_lock(&fuse_mutex);
 	err = -EINVAL;
-	if (ctx->fudptr && *ctx->fudptr)
-		goto err_unlock;
+	if (ctx->fudptr && *ctx->fudptr) {
+		if (*ctx->fudptr == FUSE_DEV_SYNC_INIT) {
+			fc->sync_init = 1;
+		} else
+			goto err_unlock;
+	}
 
 	err = fuse_ctl_add_conn(fc);
 	if (err)
@@ -1876,8 +1889,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;
 
@@ -1898,6 +1913,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
 static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
 {
 	struct fuse_fs_context *ctx = fsc->fs_private;
+	struct fuse_mount *fm;
 	int err;
 
 	if (!ctx->file || !ctx->rootmode_present ||
@@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
 		return err;
 	/* file->private_data shall be visible on all CPUs after this */
 	smp_mb();
-	fuse_send_init(get_fuse_mount_super(sb));
-	return 0;
+
+	fm = get_fuse_mount_super(sb);
+
+	if (fm->fc->sync_init) {
+		struct fuse_init_args *ia = fuse_new_init(fm);
+
+		err = fuse_simple_request(fm, &ia->args);
+		if (err > 0)
+			err = 0;
+		process_init_reply(fm, &ia->args, err);
+	} else {
+		fuse_send_init(fm);
+		err = 0;
+	}
+
+	return err;
 }
 
 /*
@@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
 	 * Allow creating a fuse mount with an already initialized fuse
 	 * connection
 	 */
-	fud = READ_ONCE(ctx->file->private_data);
+	fud = __fuse_get_dev(ctx->file);
 	if (ctx->file->f_op == &fuse_dev_operations && fud) {
 		fsc->sget_key = fud->fc;
 		sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 94621f68a5cc..6b9fb8b08768 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1131,6 +1131,7 @@ struct fuse_backing_map {
 #define FUSE_DEV_IOC_BACKING_OPEN	_IOW(FUSE_DEV_IOC_MAGIC, 1, \
 					     struct fuse_backing_map)
 #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
+#define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
 
 struct fuse_lseek_in {
 	uint64_t	fh;
-- 
2.49.0

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Bernd Schubert]

On 8/22/25 13:44, Miklos Szeredi 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.

I really like the ida. Another reason is that fuse-server might want to
abort during FUSE_INIT and it then leaves a stale mount that in worst
case might have gotten requests already.

> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> I tested this with my raw-interface tester, so no libfuse update yet.  Will
> work on that next.
> 
>  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          |  3 ++
>  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
>  include/uapi/linux/fuse.h |  1 +
>  7 files changed, 112 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index b39844d75a80..28c96961e85d 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -52,6 +52,7 @@
>  #include <linux/user_namespace.h>
>  
>  #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>  
>  #define CUSE_CONNTBL_LEN	64
>  
> @@ -547,7 +548,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
>   */
>  static int cuse_channel_release(struct inode *inode, struct file *file)
>  {
> -	struct fuse_dev *fud = file->private_data;
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  	struct cuse_conn *cc = fc_to_cc(fud->fc);
>  
>  	/* remove from the conntbl, no more access from this point on */
> 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);
> +	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);
>  
> -	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);
>  
> @@ -2343,7 +2363,7 @@ static __poll_t fuse_dev_poll(struct file *file, poll_table *wait)
>  	struct fuse_iqueue *fiq;
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  
> -	if (!fud)
> +	if (IS_ERR(fud))
>  		return EPOLLERR;
>  
>  	fiq = &fud->fc->iq;
> @@ -2490,7 +2510,7 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>  
>  int fuse_dev_release(struct inode *inode, struct file *file)
>  {
> -	struct fuse_dev *fud = fuse_get_dev(file);
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  
>  	if (fud) {
>  		struct fuse_conn *fc = fud->fc;
> @@ -2521,8 +2541,8 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
>  {
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  
> -	if (!fud)
> -		return -EPERM;
> +	if (IS_ERR(fud))
> +		return PTR_ERR(fud);
>  
>  	/* No locking - fasync_helper does its own locking */
>  	return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> @@ -2532,7 +2552,7 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
>  {
>  	struct fuse_dev *fud;
>  
> -	if (new->private_data)
> +	if (__fuse_get_dev(new))
>  		return -EINVAL;
>  
>  	fud = fuse_dev_alloc_install(fc);
> @@ -2563,7 +2583,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
>  	 * uses the same ioctl handler.
>  	 */
>  	if (fd_file(f)->f_op == file->f_op)
> -		fud = fuse_get_dev(fd_file(f));
> +		fud = __fuse_get_dev(fd_file(f));
>  
>  	res = -EINVAL;
>  	if (fud) {
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  	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);
>  	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);
> +		err = 0;
> +	}
> +	mutex_unlock(&fuse_mutex);
> +	return err;
> +}
> +
>  static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  			   unsigned long arg)
>  {
> @@ -2625,6 +2658,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  	case FUSE_DEV_IOC_BACKING_CLOSE:
>  		return fuse_dev_ioctl_backing_close(file, argp);
>  
> +	case FUSE_DEV_IOC_SYNC_INIT:
> +		return fuse_dev_ioctl_sync_init(file);
> +
>  	default:
>  		return -ENOTTY;
>  	}
> @@ -2633,7 +2669,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  #ifdef CONFIG_PROC_FS
>  static void fuse_dev_show_fdinfo(struct seq_file *seq, struct file *file)
>  {
> -	struct fuse_dev *fud = fuse_get_dev(file);
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  	if (!fud)
>  		return;
>  
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 249b210becb1..bef38ed78249 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -1139,9 +1139,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
>  		return -EINVAL;
>  
>  	fud = fuse_get_dev(cmd->file);
> -	if (!fud) {
> +	if (IS_ERR(fud)) {
>  		pr_info_ratelimited("No fuse device found\n");
> -		return -ENOTCONN;
> +		return PTR_ERR(fud);
>  	}
>  	fc = fud->fc;
>  
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 5a9bd771a319..6e8373f97040 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -12,6 +12,8 @@
>  #define FUSE_INT_REQ_BIT (1ULL << 0)
>  #define FUSE_REQ_ID_STEP (1ULL << 1)
>  
> +extern struct wait_queue_head fuse_dev_waitq;
> +
>  struct fuse_arg;
>  struct fuse_args;
>  struct fuse_pqueue;
> @@ -37,15 +39,22 @@ struct fuse_copy_state {
>  	} ring;
>  };
>  
> -static inline struct fuse_dev *fuse_get_dev(struct file *file)
> +#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> +#define FUSE_DEV_PTR_MASK (~1UL)
> +
> +static inline struct fuse_dev *__fuse_get_dev(struct file *file)
>  {
>  	/*
>  	 * Lockless access is OK, because file->private data is set
>  	 * once during mount and is valid until the file is released.
>  	 */
> -	return READ_ONCE(file->private_data);
> +	struct fuse_dev *fud = READ_ONCE(file->private_data);
> +
> +	return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
>  }
>  
> +struct fuse_dev *fuse_get_dev(struct file *file);
> +
>  unsigned int fuse_req_hash(u64 unique);
>  struct fuse_req *fuse_request_find(struct fuse_pqueue *fpq, u64 unique);
>  
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 486fa550c951..54121207cfb2 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -904,6 +904,9 @@ struct fuse_conn {
>  	/* Is link not implemented by fs? */
>  	unsigned int no_link:1;
>  
> +	/* Is synchronous FUSE_INIT allowed? */
> +	unsigned int sync_init:1;
> +
>  	/* Use io_uring for communication */
>  	unsigned int io_uring;
>  
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d26a5bc394d..d5f9f2abc569 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -7,6 +7,7 @@
>  */
>  
>  #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>  #include "dev_uring_i.h"
>  
>  #include <linux/dax.h>
> @@ -34,6 +35,7 @@ MODULE_LICENSE("GPL");
>  static struct kmem_cache *fuse_inode_cachep;
>  struct list_head fuse_conn_list;
>  DEFINE_MUTEX(fuse_mutex);
> +DECLARE_WAIT_QUEUE_HEAD(fuse_dev_waitq);
>  
>  static int set_global_limit(const char *val, const struct kernel_param *kp);
>  
> @@ -1466,7 +1468,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>  	wake_up_all(&fc->blocked_waitq);
>  }
>  
> -void fuse_send_init(struct fuse_mount *fm)
> +static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
>  {
>  	struct fuse_init_args *ia;
>  	u64 flags;
> @@ -1525,8 +1527,15 @@ void fuse_send_init(struct fuse_mount *fm)
>  	ia->args.out_args[0].value = &ia->out;
>  	ia->args.force = true;
>  	ia->args.nocreds = true;
> -	ia->args.end = process_init_reply;
>  
> +	return ia;
> +}
> +
> +void fuse_send_init(struct fuse_mount *fm)
> +{

I'm going to review more carefully later on, but how about renaming
this to fuse_send_bg_init()?

> +	struct fuse_init_args *ia = fuse_new_init(fm);
> +
> +	ia->args.end = process_init_reply;
>  	if (fuse_simple_background(fm, &ia->args, GFP_KERNEL) != 0)
>  		process_init_reply(fm, &ia->args, -ENOTCONN);



Thanks,
Bernd

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Joanne Koong]

On Fri, Aug 22, 2025 at 4:44 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>
> ---
> I tested this with my raw-interface tester, so no libfuse update yet.  Will
> work on that next.
>
>  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          |  3 ++
>  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
>  include/uapi/linux/fuse.h |  1 +
>  7 files changed, 112 insertions(+), 32 deletions(-)
>

Will read this more thoroughly next week but left some comments below for now.

> 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);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       fud = __fuse_get_dev(file);
> +       if (!fud)
> +               return ERR_PTR(-EPERM);
> +
> +       return fud;
> +}
> +
>
> +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);
> +               err = 0;
> +       }
> +       mutex_unlock(&fuse_mutex);
> +       return err;

Does this let an untrusted server deadlock fuse if they call
FUSE_DEV_IOC_SYNC_INIT twice? afaict, fuse_mutex is a global lock and
the 2nd FUSE_DEV_IOC_SYNC_INIT call can forever hold fuse_mutex
because of the __fuse_get_dev() -> wait_event_interruptible().

> +}
> +
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d26a5bc394d..d5f9f2abc569 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1898,6 +1913,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
>  static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>  {
>         struct fuse_fs_context *ctx = fsc->fs_private;
> +       struct fuse_mount *fm;
>         int err;
>
>         if (!ctx->file || !ctx->rootmode_present ||
> @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>                 return err;
>         /* file->private_data shall be visible on all CPUs after this */
>         smp_mb();
> -       fuse_send_init(get_fuse_mount_super(sb));
> -       return 0;
> +
> +       fm = get_fuse_mount_super(sb);
> +
> +       if (fm->fc->sync_init) {
> +               struct fuse_init_args *ia = fuse_new_init(fm);
> +
> +               err = fuse_simple_request(fm, &ia->args);
> +               if (err > 0)
> +                       err = 0;
> +               process_init_reply(fm, &ia->args, err);

Do we need a fuse_dev_free() here if err < 0? If err < 0 then the
mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
will have already been called which if i'm understanding it correctly
means otherwise the fc will get leaked in this case. Or I guess
another option is to retain original behavior with having the mount
succeed even if the init server reply returns back an error code?

> +       } else {
> +               fuse_send_init(fm);
> +               err = 0;
> +       }

imo this logic looks cleaner if fuse_send_init() takes in a 'bool
async' arg and the bulk of this logic is handled there. Especially if
virtio is also meant to support synchronous init requests (which I'm
not seeing why it wouldn't?)

Thanks,
Joanne

> +
> +       return err;
>  }
>
>  /*
> @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
>          * Allow creating a fuse mount with an already initialized fuse
>          * connection
>          */
> -       fud = READ_ONCE(ctx->file->private_data);
> +       fud = __fuse_get_dev(ctx->file);
>         if (ctx->file->f_op == &fuse_dev_operations && fud) {
>                 fsc->sget_key = fud->fc;
>                 sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Joanne Koong]

On Fri, Aug 22, 2025 at 3:46 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Fri, Aug 22, 2025 at 4:44 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>
> > ---
> > I tested this with my raw-interface tester, so no libfuse update yet.  Will
> > work on that next.
> >
> >  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          |  3 ++
> >  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
> >  include/uapi/linux/fuse.h |  1 +
> >  7 files changed, 112 insertions(+), 32 deletions(-)
> >
>
> Will read this more thoroughly next week but left some comments below for now.
>
> > 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);
> > +       if (err)
> > +               return ERR_PTR(err);
> > +
> > +       fud = __fuse_get_dev(file);
> > +       if (!fud)
> > +               return ERR_PTR(-EPERM);
> > +
> > +       return fud;
> > +}
> > +
> >
> > +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);
> > +               err = 0;
> > +       }
> > +       mutex_unlock(&fuse_mutex);
> > +       return err;
>
> Does this let an untrusted server deadlock fuse if they call
> FUSE_DEV_IOC_SYNC_INIT twice? afaict, fuse_mutex is a global lock and
> the 2nd FUSE_DEV_IOC_SYNC_INIT call can forever hold fuse_mutex
> because of the __fuse_get_dev() -> wait_event_interruptible().

Never mind this comment, I got __fuse_get_dev() and fuse_get_dev()
mixed up. fuse_get_dev() is the one that calls
wait_event_interruptible(), not __fuse_get_dev().

>
> > +}
> > +
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index 9d26a5bc394d..d5f9f2abc569 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1898,6 +1913,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
> >  static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> >  {
> >         struct fuse_fs_context *ctx = fsc->fs_private;
> > +       struct fuse_mount *fm;
> >         int err;
> >
> >         if (!ctx->file || !ctx->rootmode_present ||
> > @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> >                 return err;
> >         /* file->private_data shall be visible on all CPUs after this */
> >         smp_mb();
> > -       fuse_send_init(get_fuse_mount_super(sb));
> > -       return 0;
> > +
> > +       fm = get_fuse_mount_super(sb);
> > +
> > +       if (fm->fc->sync_init) {
> > +               struct fuse_init_args *ia = fuse_new_init(fm);
> > +
> > +               err = fuse_simple_request(fm, &ia->args);
> > +               if (err > 0)
> > +                       err = 0;
> > +               process_init_reply(fm, &ia->args, err);
>
> Do we need a fuse_dev_free() here if err < 0? If err < 0 then the
> mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
> will have already been called which if i'm understanding it correctly
> means otherwise the fc will get leaked in this case. Or I guess
> another option is to retain original behavior with having the mount
> succeed even if the init server reply returns back an error code?
>
> > +       } else {
> > +               fuse_send_init(fm);
> > +               err = 0;
> > +       }
>
> imo this logic looks cleaner if fuse_send_init() takes in a 'bool
> async' arg and the bulk of this logic is handled there. Especially if
> virtio is also meant to support synchronous init requests (which I'm
> not seeing why it wouldn't?)
>
> Thanks,
> Joanne
>
> > +
> > +       return err;
> >  }
> >
> >  /*
> > @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
> >          * Allow creating a fuse mount with an already initialized fuse
> >          * connection
> >          */
> > -       fud = READ_ONCE(ctx->file->private_data);
> > +       fud = __fuse_get_dev(ctx->file);
> >         if (ctx->file->f_op == &fuse_dev_operations && fud) {
> >                 fsc->sget_key = fud->fc;
> >                 sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Darrick J. Wong]

On Fri, Aug 22, 2025 at 01:44:35PM +0200, Miklos Szeredi 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>
> ---
> I tested this with my raw-interface tester, so no libfuse update yet.  Will
> work on that next.
> 
>  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          |  3 ++
>  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
>  include/uapi/linux/fuse.h |  1 +
>  7 files changed, 112 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index b39844d75a80..28c96961e85d 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -52,6 +52,7 @@
>  #include <linux/user_namespace.h>
>  
>  #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>  
>  #define CUSE_CONNTBL_LEN	64
>  
> @@ -547,7 +548,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
>   */
>  static int cuse_channel_release(struct inode *inode, struct file *file)
>  {
> -	struct fuse_dev *fud = file->private_data;
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  	struct cuse_conn *cc = fc_to_cc(fud->fc);
>  
>  	/* remove from the conntbl, no more access from this point on */
> 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);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	fud = __fuse_get_dev(file);
> +	if (!fud)
> +		return ERR_PTR(-EPERM);
> +
> +	return fud;
> +}

Heh, I was also thinking about (ab)using file->private_data for a
similar purpose of stashing flags in an open fuse dev until mount time!

In my situation, I want to split a fuse server into two pieces: a
privileged mount helper, and a no-privilege fuse server process that
parses fs metadata and responds to fuse requests.

The second process runs as a systemd socket service and has no access to
anything other than a stripped down version of the root filesystem.  The
service is started by the mount helper.  The two processes share an
AF_UNIX socket so that the fuse server can ask the mount helper to pass
it mount options and file descriptors, and when that's done, the mount
helper actually calls mount() to start up the filesystem.

Therefore, I want the mount helper to convey whatever privilege it has
(e.g. iomap, which requires CAP_SYS_RAWIO) to the fuse dev.  I think
this actually works in practice because FUSE_IOMAP is set from
fuse_send_init, which is called from fuse_fill_super, which runs in the
mount helper's process context.

So maybe I don't need to do anything, but there's one weird thing.
I also created an ioctl for /dev/fuse so that userspace can query if
it's likely to get iomap access, because there are a few things that
fuse4fs wants to do to prepare itself for fuse.

In all the fuse-iomap prototypes up through v4, fuse4fs calls mount()
call so it's one and the same, but now with the separation, I need that
decision to allow iomap to ride the /dev/fuse fd from the mount helper
into the separated fuse server.  This would be a way to do that, though
TBH hiding bits in a pointer until the ctx->fudptr assignment is ...
gross.

The other answer could be drop the CAP_SYS_RAWIO requirement from the
fuse-iomap code.

--D

> +
>  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);
>  
> -	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);
>  
> @@ -2343,7 +2363,7 @@ static __poll_t fuse_dev_poll(struct file *file, poll_table *wait)
>  	struct fuse_iqueue *fiq;
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  
> -	if (!fud)
> +	if (IS_ERR(fud))
>  		return EPOLLERR;
>  
>  	fiq = &fud->fc->iq;
> @@ -2490,7 +2510,7 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>  
>  int fuse_dev_release(struct inode *inode, struct file *file)
>  {
> -	struct fuse_dev *fud = fuse_get_dev(file);
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  
>  	if (fud) {
>  		struct fuse_conn *fc = fud->fc;
> @@ -2521,8 +2541,8 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
>  {
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  
> -	if (!fud)
> -		return -EPERM;
> +	if (IS_ERR(fud))
> +		return PTR_ERR(fud);
>  
>  	/* No locking - fasync_helper does its own locking */
>  	return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> @@ -2532,7 +2552,7 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
>  {
>  	struct fuse_dev *fud;
>  
> -	if (new->private_data)
> +	if (__fuse_get_dev(new))
>  		return -EINVAL;
>  
>  	fud = fuse_dev_alloc_install(fc);
> @@ -2563,7 +2583,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
>  	 * uses the same ioctl handler.
>  	 */
>  	if (fd_file(f)->f_op == file->f_op)
> -		fud = fuse_get_dev(fd_file(f));
> +		fud = __fuse_get_dev(fd_file(f));
>  
>  	res = -EINVAL;
>  	if (fud) {
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
>  	struct fuse_dev *fud = fuse_get_dev(file);
>  	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);
>  	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);
> +		err = 0;
> +	}
> +	mutex_unlock(&fuse_mutex);
> +	return err;
> +}
> +
>  static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  			   unsigned long arg)
>  {
> @@ -2625,6 +2658,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  	case FUSE_DEV_IOC_BACKING_CLOSE:
>  		return fuse_dev_ioctl_backing_close(file, argp);
>  
> +	case FUSE_DEV_IOC_SYNC_INIT:
> +		return fuse_dev_ioctl_sync_init(file);
> +
>  	default:
>  		return -ENOTTY;
>  	}
> @@ -2633,7 +2669,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
>  #ifdef CONFIG_PROC_FS
>  static void fuse_dev_show_fdinfo(struct seq_file *seq, struct file *file)
>  {
> -	struct fuse_dev *fud = fuse_get_dev(file);
> +	struct fuse_dev *fud = __fuse_get_dev(file);
>  	if (!fud)
>  		return;
>  
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 249b210becb1..bef38ed78249 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -1139,9 +1139,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
>  		return -EINVAL;
>  
>  	fud = fuse_get_dev(cmd->file);
> -	if (!fud) {
> +	if (IS_ERR(fud)) {
>  		pr_info_ratelimited("No fuse device found\n");
> -		return -ENOTCONN;
> +		return PTR_ERR(fud);
>  	}
>  	fc = fud->fc;
>  
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 5a9bd771a319..6e8373f97040 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -12,6 +12,8 @@
>  #define FUSE_INT_REQ_BIT (1ULL << 0)
>  #define FUSE_REQ_ID_STEP (1ULL << 1)
>  
> +extern struct wait_queue_head fuse_dev_waitq;
> +
>  struct fuse_arg;
>  struct fuse_args;
>  struct fuse_pqueue;
> @@ -37,15 +39,22 @@ struct fuse_copy_state {
>  	} ring;
>  };
>  
> -static inline struct fuse_dev *fuse_get_dev(struct file *file)
> +#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> +#define FUSE_DEV_PTR_MASK (~1UL)
> +
> +static inline struct fuse_dev *__fuse_get_dev(struct file *file)
>  {
>  	/*
>  	 * Lockless access is OK, because file->private data is set
>  	 * once during mount and is valid until the file is released.
>  	 */
> -	return READ_ONCE(file->private_data);
> +	struct fuse_dev *fud = READ_ONCE(file->private_data);
> +
> +	return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
>  }
>  
> +struct fuse_dev *fuse_get_dev(struct file *file);
> +
>  unsigned int fuse_req_hash(u64 unique);
>  struct fuse_req *fuse_request_find(struct fuse_pqueue *fpq, u64 unique);
>  
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 486fa550c951..54121207cfb2 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -904,6 +904,9 @@ struct fuse_conn {
>  	/* Is link not implemented by fs? */
>  	unsigned int no_link:1;
>  
> +	/* Is synchronous FUSE_INIT allowed? */
> +	unsigned int sync_init:1;
> +
>  	/* Use io_uring for communication */
>  	unsigned int io_uring;
>  
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d26a5bc394d..d5f9f2abc569 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -7,6 +7,7 @@
>  */
>  
>  #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>  #include "dev_uring_i.h"
>  
>  #include <linux/dax.h>
> @@ -34,6 +35,7 @@ MODULE_LICENSE("GPL");
>  static struct kmem_cache *fuse_inode_cachep;
>  struct list_head fuse_conn_list;
>  DEFINE_MUTEX(fuse_mutex);
> +DECLARE_WAIT_QUEUE_HEAD(fuse_dev_waitq);
>  
>  static int set_global_limit(const char *val, const struct kernel_param *kp);
>  
> @@ -1466,7 +1468,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>  	wake_up_all(&fc->blocked_waitq);
>  }
>  
> -void fuse_send_init(struct fuse_mount *fm)
> +static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
>  {
>  	struct fuse_init_args *ia;
>  	u64 flags;
> @@ -1525,8 +1527,15 @@ void fuse_send_init(struct fuse_mount *fm)
>  	ia->args.out_args[0].value = &ia->out;
>  	ia->args.force = true;
>  	ia->args.nocreds = true;
> -	ia->args.end = process_init_reply;
>  
> +	return ia;
> +}
> +
> +void fuse_send_init(struct fuse_mount *fm)
> +{
> +	struct fuse_init_args *ia = fuse_new_init(fm);
> +
> +	ia->args.end = process_init_reply;
>  	if (fuse_simple_background(fm, &ia->args, GFP_KERNEL) != 0)
>  		process_init_reply(fm, &ia->args, -ENOTCONN);
>  }
> @@ -1867,8 +1876,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>  
>  	mutex_lock(&fuse_mutex);
>  	err = -EINVAL;
> -	if (ctx->fudptr && *ctx->fudptr)
> -		goto err_unlock;
> +	if (ctx->fudptr && *ctx->fudptr) {
> +		if (*ctx->fudptr == FUSE_DEV_SYNC_INIT) {
> +			fc->sync_init = 1;
> +		} else
> +			goto err_unlock;
> +	}
>  
>  	err = fuse_ctl_add_conn(fc);
>  	if (err)
> @@ -1876,8 +1889,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;
>  
> @@ -1898,6 +1913,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
>  static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>  {
>  	struct fuse_fs_context *ctx = fsc->fs_private;
> +	struct fuse_mount *fm;
>  	int err;
>  
>  	if (!ctx->file || !ctx->rootmode_present ||
> @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>  		return err;
>  	/* file->private_data shall be visible on all CPUs after this */
>  	smp_mb();
> -	fuse_send_init(get_fuse_mount_super(sb));
> -	return 0;
> +
> +	fm = get_fuse_mount_super(sb);
> +
> +	if (fm->fc->sync_init) {
> +		struct fuse_init_args *ia = fuse_new_init(fm);
> +
> +		err = fuse_simple_request(fm, &ia->args);
> +		if (err > 0)
> +			err = 0;
> +		process_init_reply(fm, &ia->args, err);
> +	} else {
> +		fuse_send_init(fm);
> +		err = 0;
> +	}
> +
> +	return err;
>  }
>  
>  /*
> @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
>  	 * Allow creating a fuse mount with an already initialized fuse
>  	 * connection
>  	 */
> -	fud = READ_ONCE(ctx->file->private_data);
> +	fud = __fuse_get_dev(ctx->file);
>  	if (ctx->file->f_op == &fuse_dev_operations && fud) {
>  		fsc->sget_key = fud->fc;
>  		sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 94621f68a5cc..6b9fb8b08768 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -1131,6 +1131,7 @@ struct fuse_backing_map {
>  #define FUSE_DEV_IOC_BACKING_OPEN	_IOW(FUSE_DEV_IOC_MAGIC, 1, \
>  					     struct fuse_backing_map)
>  #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
> +#define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
>  
>  struct fuse_lseek_in {
>  	uint64_t	fh;
> -- 
> 2.49.0
> 
> 

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Darrick J. Wong]

On Fri, Aug 22, 2025 at 03:52:38PM -0700, Joanne Koong wrote:
> On Fri, Aug 22, 2025 at 3:46 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > On Fri, Aug 22, 2025 at 4:44 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>
> > > ---
> > > I tested this with my raw-interface tester, so no libfuse update yet.  Will
> > > work on that next.
> > >
> > >  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          |  3 ++
> > >  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
> > >  include/uapi/linux/fuse.h |  1 +
> > >  7 files changed, 112 insertions(+), 32 deletions(-)
> > >
> >
> > Will read this more thoroughly next week but left some comments below for now.
> >
> > > 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);
> > > +       if (err)
> > > +               return ERR_PTR(err);
> > > +
> > > +       fud = __fuse_get_dev(file);
> > > +       if (!fud)
> > > +               return ERR_PTR(-EPERM);
> > > +
> > > +       return fud;
> > > +}
> > > +
> > >
> > > +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);
> > > +               err = 0;
> > > +       }
> > > +       mutex_unlock(&fuse_mutex);
> > > +       return err;
> >
> > Does this let an untrusted server deadlock fuse if they call
> > FUSE_DEV_IOC_SYNC_INIT twice? afaict, fuse_mutex is a global lock and
> > the 2nd FUSE_DEV_IOC_SYNC_INIT call can forever hold fuse_mutex
> > because of the __fuse_get_dev() -> wait_event_interruptible().
> 
> Never mind this comment, I got __fuse_get_dev() and fuse_get_dev()
> mixed up. fuse_get_dev() is the one that calls
> wait_event_interruptible(), not __fuse_get_dev().
> 
> >
> > > +}
> > > +
> > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > > index 9d26a5bc394d..d5f9f2abc569 100644
> > > --- a/fs/fuse/inode.c
> > > +++ b/fs/fuse/inode.c
> > > @@ -1898,6 +1913,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
> > >  static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> > >  {
> > >         struct fuse_fs_context *ctx = fsc->fs_private;
> > > +       struct fuse_mount *fm;
> > >         int err;
> > >
> > >         if (!ctx->file || !ctx->rootmode_present ||
> > > @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> > >                 return err;
> > >         /* file->private_data shall be visible on all CPUs after this */
> > >         smp_mb();
> > > -       fuse_send_init(get_fuse_mount_super(sb));
> > > -       return 0;
> > > +
> > > +       fm = get_fuse_mount_super(sb);
> > > +
> > > +       if (fm->fc->sync_init) {
> > > +               struct fuse_init_args *ia = fuse_new_init(fm);
> > > +
> > > +               err = fuse_simple_request(fm, &ia->args);
> > > +               if (err > 0)
> > > +                       err = 0;
> > > +               process_init_reply(fm, &ia->args, err);
> >
> > Do we need a fuse_dev_free() here if err < 0? If err < 0 then the

Er... are you asking if we should drop the newly created fud via
fuse_dev_release if err != 0?  (AFAICT there is no fuse_dev_free?)

> > mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
> > will have already been called which if i'm understanding it correctly
> > means otherwise the fc will get leaked in this case. Or I guess
> > another option is to retain original behavior with having the mount
> > succeed even if the init server reply returns back an error code?

<shrug> I was figuring that it was fine to leave the fud attached to the
device fd until the caller close()s it, but OTOH maybe the fuse server
would like to try to mount again?  Do fuse servers do that?

--D

> > > +       } else {
> > > +               fuse_send_init(fm);
> > > +               err = 0;
> > > +       }
> >
> > imo this logic looks cleaner if fuse_send_init() takes in a 'bool
> > async' arg and the bulk of this logic is handled there. Especially if
> > virtio is also meant to support synchronous init requests (which I'm
> > not seeing why it wouldn't?)
> >
> > Thanks,
> > Joanne
> >
> > > +
> > > +       return err;
> > >  }
> > >
> > >  /*
> > > @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
> > >          * Allow creating a fuse mount with an already initialized fuse
> > >          * connection
> > >          */
> > > -       fud = READ_ONCE(ctx->file->private_data);
> > > +       fud = __fuse_get_dev(ctx->file);
> > >         if (ctx->file->f_op == &fuse_dev_operations && fud) {
> > >                 fsc->sget_key = fud->fc;
> > >                 sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
> 

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Joanne Koong]

On Tue, Aug 26, 2025 at 12:26 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Fri, Aug 22, 2025 at 03:52:38PM -0700, Joanne Koong wrote:
> > On Fri, Aug 22, 2025 at 3:46 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > On Fri, Aug 22, 2025 at 4:44 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>
> > > > ---
> > > > I tested this with my raw-interface tester, so no libfuse update yet.  Will
> > > > work on that next.
> > > >
> > > >  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          |  3 ++
> > > >  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
> > > >  include/uapi/linux/fuse.h |  1 +
> > > >  7 files changed, 112 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > > > index 9d26a5bc394d..d5f9f2abc569 100644
> > > > --- a/fs/fuse/inode.c
> > > > +++ b/fs/fuse/inode.c
> > > > @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> > > >                 return err;
> > > >         /* file->private_data shall be visible on all CPUs after this */
> > > >         smp_mb();
> > > > -       fuse_send_init(get_fuse_mount_super(sb));
> > > > -       return 0;
> > > > +
> > > > +       fm = get_fuse_mount_super(sb);
> > > > +
> > > > +       if (fm->fc->sync_init) {
> > > > +               struct fuse_init_args *ia = fuse_new_init(fm);
> > > > +
> > > > +               err = fuse_simple_request(fm, &ia->args);
> > > > +               if (err > 0)
> > > > +                       err = 0;
> > > > +               process_init_reply(fm, &ia->args, err);
> > >
> > > Do we need a fuse_dev_free() here if err < 0? If err < 0 then the
>
> Er... are you asking if we should drop the newly created fud via
> fuse_dev_release if err != 0?  (AFAICT there is no fuse_dev_free?)
>

That's weird, I see fuse_dev_free() in fs/fuse/inode.c (eg
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fuse/inode.c#n1624)

> > > mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
> > > will have already been called which if i'm understanding it correctly
> > > means otherwise the fc will get leaked in this case. Or I guess
> > > another option is to retain original behavior with having the mount
> > > succeed even if the init server reply returns back an error code?
>
> <shrug> I was figuring that it was fine to leave the fud attached to the
> device fd until the caller close()s it, but OTOH maybe the fuse server
> would like to try to mount again?  Do fuse servers do that?

Won't this still leak the reference? From what I see, the mount will
create the fc (refcount 1) then when the mount does the dev
installation (fuse_dev_install()) that'll acquire another reference on
fc. With the caller close()ing it, that releases 1 refcount but
there's still 1 refcount that needs to be released by
fuse_mount_destroy(). As I understand it, if the mount fails, the
.kill_sb -> fuse_mount_destroy() never gets triggered (since it was
never mounted) which will leave 1 refcount remaining.

>
> --D
>

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Joanne Koong]

On Tue, Aug 26, 2025 at 2:36 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Tue, Aug 26, 2025 at 12:26 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Fri, Aug 22, 2025 at 03:52:38PM -0700, Joanne Koong wrote:
> > > On Fri, Aug 22, 2025 at 3:46 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > >
> > > > On Fri, Aug 22, 2025 at 4:44 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>
> > > > > ---
> > > > > I tested this with my raw-interface tester, so no libfuse update yet.  Will
> > > > > work on that next.
> > > > >
> > > > >  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          |  3 ++
> > > > >  fs/fuse/inode.c           | 46 +++++++++++++++++++-----
> > > > >  include/uapi/linux/fuse.h |  1 +
> > > > >  7 files changed, 112 insertions(+), 32 deletions(-)
> > > > >
> > > > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > > > > index 9d26a5bc394d..d5f9f2abc569 100644
> > > > > --- a/fs/fuse/inode.c
> > > > > +++ b/fs/fuse/inode.c
> > > > > @@ -1918,8 +1934,22 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> > > > >                 return err;
> > > > >         /* file->private_data shall be visible on all CPUs after this */
> > > > >         smp_mb();
> > > > > -       fuse_send_init(get_fuse_mount_super(sb));
> > > > > -       return 0;
> > > > > +
> > > > > +       fm = get_fuse_mount_super(sb);
> > > > > +
> > > > > +       if (fm->fc->sync_init) {
> > > > > +               struct fuse_init_args *ia = fuse_new_init(fm);
> > > > > +
> > > > > +               err = fuse_simple_request(fm, &ia->args);
> > > > > +               if (err > 0)
> > > > > +                       err = 0;
> > > > > +               process_init_reply(fm, &ia->args, err);
> > > >
> > > > Do we need a fuse_dev_free() here if err < 0? If err < 0 then the
> >
> > Er... are you asking if we should drop the newly created fud via
> > fuse_dev_release if err != 0?  (AFAICT there is no fuse_dev_free?)
> >
>
> That's weird, I see fuse_dev_free() in fs/fuse/inode.c (eg
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fuse/inode.c#n1624)
>
> > > > mount fails, but fuse_fill_super_common() -> fuse_dev_alloc_install()
> > > > will have already been called which if i'm understanding it correctly
> > > > means otherwise the fc will get leaked in this case. Or I guess
> > > > another option is to retain original behavior with having the mount
> > > > succeed even if the init server reply returns back an error code?
> >
> > <shrug> I was figuring that it was fine to leave the fud attached to the
> > device fd until the caller close()s it, but OTOH maybe the fuse server
> > would like to try to mount again?  Do fuse servers do that?
>
> Won't this still leak the reference? From what I see, the mount will
> create the fc (refcount 1) then when the mount does the dev
> installation (fuse_dev_install()) that'll acquire another reference on
> fc. With the caller close()ing it, that releases 1 refcount but
> there's still 1 refcount that needs to be released by
> fuse_mount_destroy(). As I understand it, if the mount fails, the
> .kill_sb -> fuse_mount_destroy() never gets triggered (since it was
> never mounted) which will leave 1 refcount remaining.

Ahh okay, I just tested this experimentally and I see now where I'm
wrong. If the mount fails, the .kill_sb -> fuse_mount_destroy()
callback does actually still get triggered.

>
> >
> > --D
> >

Re: [PATCH] fuse: allow synchronous FUSE_INIT

[Miklos Szeredi]

On Tue, Aug 26, 2025 at 11:44 PM Joanne Koong <joannelkoong@gmail.com> wrote:

> Ahh okay, I just tested this experimentally and I see now where I'm
> wrong. If the mount fails, the .kill_sb -> fuse_mount_destroy()
> callback does actually still get triggered.

You're not alone in getting confused over this.  Super block
initialization and cleanup are complicated sequences and not hard to
mess up.  Thanks for confirming that it works as intended.

Thanks,
Miklos