[PATCH v2 0/6] fuse: fix hang with sync init

[PATCH v2 0/6] fuse: fix hang with sync init

[Miklos Szeredi]

This is better polished, still needs more testing.

---
Miklos Szeredi (6):
  fuse: create fuse_dev on /dev/fuse open instead of mount
  fuse: add refcount to fuse_dev
  fuse: don't require /dev/fuse fd to be kept open during mount
  fuse: clean up device cloning
  fuse: alloc pqueue before installing fc
  fuse: support FSCONFIG_SET_FD for "fd" option

 fs/fuse/cuse.c       |   2 +-
 fs/fuse/dev.c        |  89 ++++++++++------------
 fs/fuse/dev_uring.c  |   2 +-
 fs/fuse/fuse_dev_i.h |  30 ++++++--
 fs/fuse/fuse_i.h     |  21 +++--
 fs/fuse/inode.c      | 177 ++++++++++++++++++++++++++-----------------
 fs/fuse/virtio_fs.c  |   8 +-
 7 files changed, 181 insertions(+), 148 deletions(-)

-- 
2.53.0

[PATCH v2 1/6] fuse: create fuse_dev on /dev/fuse open instead of mount

[Miklos Szeredi]

Allocate struct fuse_dev when opening the device.  This means that unlike
before, ->private_data is always set to a valid pointer.

The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
replaced with a simple bool sync_init member.

If sync INIT is not set, I/O on the device returns error before mount.
Keep this behavior by checking for the ->fc member.  If fud->fc is set, the
mount has succeeded.  Testing this used READ_ONCE(file->private_data) and
smp_mb() to try and provide the necessary semantics.  Switch this to
smp_store_release() and smp_load_acquire().

Setting fud->fc is protected by fuse_mutex, this is unchanged.

Will need this later so the /dev/fuse open file reference is not held
during FSCONFIG_CMD_CREATE.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dev.c        | 47 +++++++++++++++++---------------------------
 fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++--------
 fs/fuse/fuse_i.h     |  5 ++---
 fs/fuse/inode.c      | 35 +++++++++++----------------------
 fs/fuse/virtio_fs.c  |  2 --
 5 files changed, 56 insertions(+), 66 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 2c16b94357d5..4795e3d01b75 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1542,32 +1542,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 
 static int fuse_dev_open(struct inode *inode, struct file *file)
 {
-	/*
-	 * The fuse device's file's private_data is used to hold
-	 * the fuse_conn(ection) when it is mounted, and is used to
-	 * keep track of whether the file has been mounted already.
-	 */
-	file->private_data = NULL;
+	struct fuse_dev *fud = fuse_dev_alloc();
+
+	if (!fud)
+		return -ENOMEM;
+
+	file->private_data = fud;
 	return 0;
 }
 
 struct fuse_dev *fuse_get_dev(struct file *file)
 {
-	struct fuse_dev *fud = __fuse_get_dev(file);
+	struct fuse_dev *fud = fuse_file_to_fud(file);
 	int err;
 
-	if (likely(fud))
-		return fud;
-
-	err = wait_event_interruptible(fuse_dev_waitq,
-				       READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
+	err = wait_event_interruptible(fuse_dev_waitq, fuse_dev_fc_get(fud));
 	if (err)
 		return ERR_PTR(err);
 
-	fud = __fuse_get_dev(file);
-	if (!fud)
-		return ERR_PTR(-EPERM);
-
 	return fud;
 }
 
@@ -2534,10 +2526,10 @@ 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_file_to_fud(file);
+	struct fuse_conn *fc = fuse_dev_fc_get(fud);
 
-	if (fud) {
-		struct fuse_conn *fc = fud->fc;
+	if (fc) {
 		struct fuse_pqueue *fpq = &fud->pq;
 		LIST_HEAD(to_end);
 		unsigned int i;
@@ -2555,8 +2547,8 @@ int fuse_dev_release(struct inode *inode, struct file *file)
 			WARN_ON(fc->iq.fasync != NULL);
 			fuse_abort_conn(fc);
 		}
-		fuse_dev_free(fud);
 	}
+	fuse_dev_free(fud);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fuse_dev_release);
@@ -2574,16 +2566,12 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
 
 static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
 {
-	struct fuse_dev *fud;
+	struct fuse_dev *new_fud = fuse_file_to_fud(new);
 
-	if (__fuse_get_dev(new))
+	if (fuse_dev_fc_get(new_fud))
 		return -EINVAL;
 
-	fud = fuse_dev_alloc_install(fc);
-	if (!fud)
-		return -ENOMEM;
-
-	new->private_data = fud;
+	fuse_dev_install(new_fud, fc);
 	atomic_inc(&fc->dev_count);
 
 	return 0;
@@ -2657,10 +2645,11 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
 static long fuse_dev_ioctl_sync_init(struct file *file)
 {
 	int err = -EINVAL;
+	struct fuse_dev *fud = fuse_file_to_fud(file);
 
 	mutex_lock(&fuse_mutex);
-	if (!__fuse_get_dev(file)) {
-		WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
+	if (!fuse_dev_fc_get(fud)) {
+		fud->sync_init = true;
 		err = 0;
 	}
 	mutex_unlock(&fuse_mutex);
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 134bf44aff0d..522b2012cd1f 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -39,18 +39,35 @@ struct fuse_copy_state {
 	} ring;
 };
 
-#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
-#define FUSE_DEV_PTR_MASK (~1UL)
+/*
+ * Lockless access is OK, because fud->fc is set once during mount and is valid
+ * until the file is released.
+ */
+static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
+{
+	/* Pairs with smp_store_release() in fuse_dev_fc_set() */
+	return smp_load_acquire(&fud->fc);
+}
+
+static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
+{
+	/* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
+	smp_store_release(&fud->fc, fc);
+}
+
+static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
+{
+	return file->private_data;
+}
 
 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.
-	 */
-	struct fuse_dev *fud = READ_ONCE(file->private_data);
+	struct fuse_dev *fud = fuse_file_to_fud(file);
+
+	if (!fuse_dev_fc_get(fud))
+		return NULL;
 
-	return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
+	return fud;
 }
 
 struct fuse_dev *fuse_get_dev(struct file *file);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..739220d96b6f 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -576,6 +576,8 @@ struct fuse_pqueue {
  * Fuse device instance
  */
 struct fuse_dev {
+	bool sync_init;
+
 	/** Fuse connection for this device */
 	struct fuse_conn *fc;
 
@@ -622,9 +624,6 @@ struct fuse_fs_context {
 
 	/* DAX device, may be NULL */
 	struct dax_device *dax_dev;
-
-	/* fuse_dev pointer to fill in, should contain NULL on entry */
-	void **fudptr;
 };
 
 struct fuse_sync_bucket {
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e57b8af06be9..53663846db36 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1637,7 +1637,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc);
 
 void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
 {
-	fud->fc = fuse_conn_get(fc);
+	fuse_dev_fc_set(fud, fuse_conn_get(fc));
 	spin_lock(&fc->lock);
 	list_add_tail(&fud->entry, &fc->devices);
 	spin_unlock(&fc->lock);
@@ -1659,7 +1659,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
 
 void fuse_dev_free(struct fuse_dev *fud)
 {
-	struct fuse_conn *fc = fud->fc;
+	struct fuse_conn *fc = fuse_dev_fc_get(fud);
 
 	if (fc) {
 		spin_lock(&fc->lock);
@@ -1822,7 +1822,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
 
 int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 {
-	struct fuse_dev *fud = NULL;
+	struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	struct fuse_conn *fc = fm->fc;
 	struct inode *root;
@@ -1856,18 +1856,11 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 			goto err;
 	}
 
-	if (ctx->fudptr) {
-		err = -ENOMEM;
-		fud = fuse_dev_alloc_install(fc);
-		if (!fud)
-			goto err_free_dax;
-	}
-
 	fc->dev = sb->s_dev;
 	fm->sb = sb;
 	err = fuse_bdi_init(fc, sb);
 	if (err)
-		goto err_dev_free;
+		goto err_free_dax;
 
 	/* Handle umasking inside the fuse code */
 	if (sb->s_flags & SB_POSIXACL)
@@ -1889,15 +1882,15 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 	set_default_d_op(sb, &fuse_dentry_operations);
 	root_dentry = d_make_root(root);
 	if (!root_dentry)
-		goto err_dev_free;
+		goto err_free_dax;
 
 	mutex_lock(&fuse_mutex);
 	err = -EINVAL;
-	if (ctx->fudptr && *ctx->fudptr) {
-		if (*ctx->fudptr == FUSE_DEV_SYNC_INIT)
-			fc->sync_init = 1;
-		else
+	if (fud) {
+		if (fuse_dev_fc_get(fud))
 			goto err_unlock;
+		if (fud->sync_init)
+			fc->sync_init = 1;
 	}
 
 	err = fuse_ctl_add_conn(fc);
@@ -1906,8 +1899,8 @@ 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) {
-		*ctx->fudptr = fud;
+	if (fud) {
+		fuse_dev_install(fud, fc);
 		wake_up_all(&fuse_dev_waitq);
 	}
 	mutex_unlock(&fuse_mutex);
@@ -1916,9 +1909,6 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
  err_unlock:
 	mutex_unlock(&fuse_mutex);
 	dput(root_dentry);
- err_dev_free:
-	if (fud)
-		fuse_dev_free(fud);
  err_free_dax:
 	if (IS_ENABLED(CONFIG_FUSE_DAX))
 		fuse_dax_conn_free(fc);
@@ -1944,13 +1934,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
 	if ((ctx->file->f_op != &fuse_dev_operations) ||
 	    (ctx->file->f_cred->user_ns != sb->s_user_ns))
 		return -EINVAL;
-	ctx->fudptr = &ctx->file->private_data;
 
 	err = fuse_fill_super_common(sb, ctx);
 	if (err)
 		return err;
-	/* file->private_data shall be visible on all CPUs after this */
-	smp_mb();
 
 	fm = get_fuse_mount_super(sb);
 
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 2f7485ffac52..f685916754ad 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1590,8 +1590,6 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
 			goto err_free_fuse_devs;
 	}
 
-	/* virtiofs allocates and installs its own fuse devices */
-	ctx->fudptr = NULL;
 	if (ctx->dax_mode != FUSE_DAX_NEVER) {
 		if (ctx->dax_mode == FUSE_DAX_ALWAYS && !fs->dax_dev) {
 			err = -EINVAL;
-- 
2.53.0

[PATCH v2 2/6] fuse: add refcount to fuse_dev

[Miklos Szeredi]

This will make it possible to grab the fuse_dev and subsequently release
the file that it came from.

In the above case, fud->fc will be set to FUSE_DEV_FC_DISCONNECTED to
indicate that this is no longer a functional device.

When trying to assign an fc to such a disconnected fuse_dev, the fc is set
to the disconnected state.

Use atomic operations xchg() and cmpxchg() to prevent races.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/cuse.c       |  2 +-
 fs/fuse/dev.c        |  9 +++++++--
 fs/fuse/fuse_dev_i.h | 11 ++++-------
 fs/fuse/fuse_i.h     |  6 ++++--
 fs/fuse/inode.c      | 43 +++++++++++++++++++++++++++++++++++--------
 fs/fuse/virtio_fs.c  |  2 +-
 6 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index dfcb98a654d8..174333633471 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -527,7 +527,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
 	cc->fc.initialized = 1;
 	rc = cuse_send_init(cc);
 	if (rc) {
-		fuse_dev_free(fud);
+		fuse_dev_put(fud);
 		return rc;
 	}
 	file->private_data = fud;
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4795e3d01b75..3adf6bd38c9b 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2527,7 +2527,8 @@ void fuse_wait_aborted(struct fuse_conn *fc)
 int fuse_dev_release(struct inode *inode, struct file *file)
 {
 	struct fuse_dev *fud = fuse_file_to_fud(file);
-	struct fuse_conn *fc = fuse_dev_fc_get(fud);
+	/* Pairs with cmpxchg() in fuse_dev_install() */
+	struct fuse_conn *fc = xchg(&fud->fc, FUSE_DEV_FC_DISCONNECTED);
 
 	if (fc) {
 		struct fuse_pqueue *fpq = &fud->pq;
@@ -2547,8 +2548,12 @@ int fuse_dev_release(struct inode *inode, struct file *file)
 			WARN_ON(fc->iq.fasync != NULL);
 			fuse_abort_conn(fc);
 		}
+		spin_lock(&fc->lock);
+		list_del(&fud->entry);
+		spin_unlock(&fc->lock);
+		fuse_conn_put(fc);
 	}
-	fuse_dev_free(fud);
+	fuse_dev_put(fud);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fuse_dev_release);
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 522b2012cd1f..9eae14ace73c 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -39,22 +39,19 @@ struct fuse_copy_state {
 	} ring;
 };
 
+/* fud->fc gets assigned to this value when /dev/fuse is closed */
+#define FUSE_DEV_FC_DISCONNECTED ((struct fuse_conn *) 1)
+
 /*
  * Lockless access is OK, because fud->fc is set once during mount and is valid
  * until the file is released.
  */
 static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
 {
-	/* Pairs with smp_store_release() in fuse_dev_fc_set() */
+	/* Pairs with xchg() in fuse_dev_install() */
 	return smp_load_acquire(&fud->fc);
 }
 
-static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
-{
-	/* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
-	smp_store_release(&fud->fc, fc);
-}
-
 static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
 {
 	return file->private_data;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 739220d96b6f..66ec92f06497 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -576,6 +576,8 @@ struct fuse_pqueue {
  * Fuse device instance
  */
 struct fuse_dev {
+	refcount_t ref;
+
 	bool sync_init;
 
 	/** Fuse connection for this device */
@@ -1341,8 +1343,8 @@ void fuse_conn_put(struct fuse_conn *fc);
 
 struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
 struct fuse_dev *fuse_dev_alloc(void);
-void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
-void fuse_dev_free(struct fuse_dev *fud);
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
+void fuse_dev_put(struct fuse_dev *fud);
 int fuse_send_init(struct fuse_mount *fm);
 
 /**
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 53663846db36..ba845092e7f2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1622,6 +1622,7 @@ struct fuse_dev *fuse_dev_alloc(void)
 	if (!fud)
 		return NULL;
 
+	refcount_set(&fud->ref, 1);
 	pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
 	if (!pq) {
 		kfree(fud);
@@ -1635,12 +1636,32 @@ struct fuse_dev *fuse_dev_alloc(void)
 }
 EXPORT_SYMBOL_GPL(fuse_dev_alloc);
 
-void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
 {
-	fuse_dev_fc_set(fud, fuse_conn_get(fc));
+	struct fuse_conn *old_fc;
+
+	fuse_conn_get(fc);
 	spin_lock(&fc->lock);
+	/*
+	 * Pairs with:
+	 *  - xchg() in fuse_dev_release()
+	 *  - smp_load_acquire() in fuse_dev_fc_get()
+	 */
+	old_fc = cmpxchg(&fud->fc, NULL, fc);
+	if (old_fc) {
+		/*
+		 * failed to set fud->fc because
+		 *  - it was already set to a different fc
+		 *  - it was set to disconneted
+		 */
+		spin_unlock(&fc->lock);
+		fuse_conn_put(fc);
+		return false;
+	}
 	list_add_tail(&fud->entry, &fc->devices);
 	spin_unlock(&fc->lock);
+
+	return true;
 }
 EXPORT_SYMBOL_GPL(fuse_dev_install);
 
@@ -1657,11 +1678,15 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
 }
 EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
 
-void fuse_dev_free(struct fuse_dev *fud)
+void fuse_dev_put(struct fuse_dev *fud)
 {
-	struct fuse_conn *fc = fuse_dev_fc_get(fud);
+	struct fuse_conn *fc;
 
-	if (fc) {
+	if (!refcount_dec_and_test(&fud->ref))
+		return;
+
+	fc = fuse_dev_fc_get(fud);
+	if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {
 		spin_lock(&fc->lock);
 		list_del(&fud->entry);
 		spin_unlock(&fc->lock);
@@ -1671,7 +1696,7 @@ void fuse_dev_free(struct fuse_dev *fud)
 	kfree(fud->pq.processing);
 	kfree(fud);
 }
-EXPORT_SYMBOL_GPL(fuse_dev_free);
+EXPORT_SYMBOL_GPL(fuse_dev_put);
 
 static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
 				      const struct fuse_inode *fi)
@@ -1900,8 +1925,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 (fud) {
-		fuse_dev_install(fud, fc);
-		wake_up_all(&fuse_dev_waitq);
+		if (!fuse_dev_install(fud, fc))
+			fc->connected = 0; /* device file got closed */
+		else
+			wake_up_all(&fuse_dev_waitq);
 	}
 	mutex_unlock(&fuse_mutex);
 	return 0;
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index f685916754ad..12300651a0f1 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -486,7 +486,7 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
 		if (!fsvq->fud)
 			continue;
 
-		fuse_dev_free(fsvq->fud);
+		fuse_dev_put(fsvq->fud);
 		fsvq->fud = NULL;
 	}
 }
-- 
2.53.0

[PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Miklos Szeredi]

With the new mount API the sequence of syscalls would be:

        fs_fd = fsopen("fuse", 0);
	snprintf(opt, sizeof(opt), "%i", devfd);
	fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", opt, 0);
	/* ... */
	fsconfig(fs_fd, FSCONFIG_CMD_CREATE, 0, 0, 0);

Current mount code just stores the value of devfd in the fs_context and
uses it in during FSCONFIG_CMD_CREATE.

This is not very elegant, but there's a bigger problem: when sync init is
used and the server exits for some reason (error, crash) while processing
FUSE_INIT, the filesystem creation will hang.  The reason is that while all
other threads will exit, the mounting thread (or process) will keep the
device fd open, which will prevent an abort from happening.

This is a regression from the async mount case, where the mount was done
first, and the FUSE_INIT processing afterwards, in which case there's no
such recursive syscall keeping the fd open.

The solution is twofold:

 a) use unshare(CLONE_FILES) in the mounting thread and close the device fd
    after fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", ...)

 b) only reference the fuse_dev from fs_context not the device file itself

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/fuse_i.h |  4 +---
 fs/fuse/inode.c  | 54 +++++++++++++++++++++++++++---------------------
 2 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 66ec92f06497..b77b384b0385 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -603,13 +603,11 @@ static inline bool fuse_is_inode_dax_mode(enum fuse_dax_mode mode)
 }
 
 struct fuse_fs_context {
-	int fd;
-	struct file *file;
+	struct fuse_dev *fud;
 	unsigned int rootmode;
 	kuid_t user_id;
 	kgid_t group_id;
 	bool is_bdev:1;
-	bool fd_present:1;
 	bool rootmode_present:1;
 	bool user_id_present:1;
 	bool group_id_present:1;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index ba845092e7f2..45abcfec03a4 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -800,6 +800,26 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
 	{}
 };
 
+static int fuse_opt_fd(struct fs_context *fsc, int fd)
+{
+	struct file *file __free(fput) = fget(fd);
+	struct fuse_fs_context *ctx = fsc->fs_private;
+
+	if (file->f_op != &fuse_dev_operations)
+		return invalfc(fsc, "fd is not a fuse device");
+	/*
+	 * Require mount to happen from the same user namespace which
+	 * opened /dev/fuse to prevent potential attacks.
+	 */
+	if (file->f_cred->user_ns != fsc->user_ns)
+		return invalfc(fsc, "wrong user namespace for fuse device");
+
+	ctx->fud = file->private_data;
+	refcount_inc(&ctx->fud->ref);
+
+	return 0;
+}
+
 static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
 {
 	struct fs_parse_result result;
@@ -839,9 +859,7 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
 		return 0;
 
 	case OPT_FD:
-		ctx->fd = result.uint_32;
-		ctx->fd_present = true;
-		break;
+		return fuse_opt_fd(fsc, result.uint_32);
 
 	case OPT_ROOTMODE:
 		if (!fuse_valid_type(result.uint_32))
@@ -904,6 +922,8 @@ static void fuse_free_fsc(struct fs_context *fsc)
 	struct fuse_fs_context *ctx = fsc->fs_private;
 
 	if (ctx) {
+		if (ctx->fud)
+			fuse_dev_put(ctx->fud);
 		kfree(ctx->subtype);
 		kfree(ctx);
 	}
@@ -1847,7 +1867,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
 
 int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 {
-	struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
+	struct fuse_dev *fud = ctx->fud;
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	struct fuse_conn *fc = fm->fc;
 	struct inode *root;
@@ -1950,18 +1970,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
 	struct fuse_mount *fm;
 	int err;
 
-	if (!ctx->file || !ctx->rootmode_present ||
+	if (!ctx->fud || !ctx->rootmode_present ||
 	    !ctx->user_id_present || !ctx->group_id_present)
 		return -EINVAL;
 
-	/*
-	 * Require mount to happen from the same user namespace which
-	 * opened /dev/fuse to prevent potential attacks.
-	 */
-	if ((ctx->file->f_op != &fuse_dev_operations) ||
-	    (ctx->file->f_cred->user_ns != sb->s_user_ns))
-		return -EINVAL;
-
 	err = fuse_fill_super_common(sb, ctx);
 	if (err)
 		return err;
@@ -1989,8 +2001,7 @@ static int fuse_test_super(struct super_block *sb, struct fs_context *fsc)
 static int fuse_get_tree(struct fs_context *fsc)
 {
 	struct fuse_fs_context *ctx = fsc->fs_private;
-	struct fuse_dev *fud;
-	struct fuse_conn *fc;
+	struct fuse_conn *fc, *key;
 	struct fuse_mount *fm;
 	struct super_block *sb;
 	int err;
@@ -2010,9 +2021,6 @@ static int fuse_get_tree(struct fs_context *fsc)
 
 	fsc->s_fs_info = fm;
 
-	if (ctx->fd_present)
-		ctx->file = fget(ctx->fd);
-
 	if (IS_ENABLED(CONFIG_BLOCK) && ctx->is_bdev) {
 		err = get_tree_bdev(fsc, fuse_fill_super);
 		goto out;
@@ -2022,16 +2030,16 @@ static int fuse_get_tree(struct fs_context *fsc)
 	 * (found by device name), normal fuse mounts can't
 	 */
 	err = -EINVAL;
-	if (!ctx->file)
+	if (!ctx->fud)
 		goto out;
 
 	/*
 	 * Allow creating a fuse mount with an already initialized fuse
 	 * connection
 	 */
-	fud = __fuse_get_dev(ctx->file);
-	if (ctx->file->f_op == &fuse_dev_operations && fud) {
-		fsc->sget_key = fud->fc;
+	key = fuse_dev_fc_get(ctx->fud);
+	if (key) {
+		fsc->sget_key = key;
 		sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
 		err = PTR_ERR_OR_ZERO(sb);
 		if (!IS_ERR(sb))
@@ -2042,8 +2050,6 @@ static int fuse_get_tree(struct fs_context *fsc)
 out:
 	if (fsc->s_fs_info)
 		fuse_mount_destroy(fm);
-	if (ctx->file)
-		fput(ctx->file);
 	return err;
 }
 
-- 
2.53.0

[PATCH v2 4/6] fuse: clean up device cloning

[Miklos Szeredi]

 - fuse_mutex is not needed for device cloning, because fuse_dev_install()
   uses cmpxcg() to set fud->fc, which prevents races between clone/mount
   or clone/clone.  This makes the logic simpler

 - Drop fc->dev_count.  This is only used to check in release if the device
   is the last clone, but checking list_empty(&fc->devices) is equivalent
   after removing the released device from the list.  Removing the fuse_dev
   before calling fuse_abort_conn() is okay, since the processing and io
   lists are now empty for this device.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dev.c    | 44 ++++++++++++++++----------------------------
 fs/fuse/fuse_i.h |  3 ---
 fs/fuse/inode.c  |  1 -
 3 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3adf6bd38c9b..18cc844cf290 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2543,14 +2543,15 @@ int fuse_dev_release(struct inode *inode, struct file *file)
 
 		fuse_dev_end_requests(&to_end);
 
+		spin_lock(&fc->lock);
+		list_del(&fud->entry);
+		spin_unlock(&fc->lock);
+
 		/* Are we the last open device? */
-		if (atomic_dec_and_test(&fc->dev_count)) {
+		if (list_empty(&fc->devices)) {
 			WARN_ON(fc->iq.fasync != NULL);
 			fuse_abort_conn(fc);
 		}
-		spin_lock(&fc->lock);
-		list_del(&fud->entry);
-		spin_unlock(&fc->lock);
 		fuse_conn_put(fc);
 	}
 	fuse_dev_put(fud);
@@ -2569,24 +2570,10 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
 	return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
 }
 
-static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
-{
-	struct fuse_dev *new_fud = fuse_file_to_fud(new);
-
-	if (fuse_dev_fc_get(new_fud))
-		return -EINVAL;
-
-	fuse_dev_install(new_fud, fc);
-	atomic_inc(&fc->dev_count);
-
-	return 0;
-}
-
 static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
 {
-	int res;
 	int oldfd;
-	struct fuse_dev *fud = NULL;
+	struct fuse_dev *fud, *new_fud;
 
 	if (get_user(oldfd, argp))
 		return -EFAULT;
@@ -2599,17 +2586,18 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
 	 * Check against file->f_op because CUSE
 	 * uses the same ioctl handler.
 	 */
-	if (fd_file(f)->f_op == file->f_op)
-		fud = __fuse_get_dev(fd_file(f));
+	if (fd_file(f)->f_op != file->f_op)
+		return -EINVAL;
 
-	res = -EINVAL;
-	if (fud) {
-		mutex_lock(&fuse_mutex);
-		res = fuse_device_clone(fud->fc, file);
-		mutex_unlock(&fuse_mutex);
-	}
+	fud = __fuse_get_dev(fd_file(f));
+	if (!fud)
+		return -EINVAL;
 
-	return res;
+	new_fud = fuse_file_to_fud(file);
+	if (!fuse_dev_install(new_fud, fud->fc))
+		return -EINVAL;
+
+	return 0;
 }
 
 static long fuse_dev_ioctl_backing_open(struct file *file,
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index b77b384b0385..92576e28f8ac 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -647,9 +647,6 @@ struct fuse_conn {
 	/** Refcount */
 	refcount_t count;
 
-	/** Number of fuse_dev's */
-	atomic_t dev_count;
-
 	/** Current epoch for up-to-date dentries */
 	atomic_t epoch;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 45abcfec03a4..e42356d60f7a 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -995,7 +995,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	spin_lock_init(&fc->bg_lock);
 	init_rwsem(&fc->killsb);
 	refcount_set(&fc->count, 1);
-	atomic_set(&fc->dev_count, 1);
 	atomic_set(&fc->epoch, 1);
 	INIT_WORK(&fc->epoch_work, fuse_epoch_work);
 	init_waitqueue_head(&fc->blocked_waitq);
-- 
2.53.0

[PATCH v2 5/6] fuse: alloc pqueue before installing fc

[Miklos Szeredi]

Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated on
mount.  But now fuse_dev is allocated when opening /dev/fuse, even though
the queues are not needed at that time.

Delay allocation of the pqueue (4k worth of list_head) just before mounting
or cloning a device.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dev.c       |  9 ++++++--
 fs/fuse/dev_uring.c |  2 +-
 fs/fuse/fuse_i.h    |  5 +++--
 fs/fuse/inode.c     | 51 +++++++++++++++++++++++++++++----------------
 fs/fuse/virtio_fs.c |  4 ++--
 5 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 18cc844cf290..4d11750addbe 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1542,7 +1542,7 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 
 static int fuse_dev_open(struct inode *inode, struct file *file)
 {
-	struct fuse_dev *fud = fuse_dev_alloc();
+	struct fuse_dev *fud = fuse_dev_alloc(false);
 
 	if (!fud)
 		return -ENOMEM;
@@ -2574,6 +2574,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
 {
 	int oldfd;
 	struct fuse_dev *fud, *new_fud;
+	struct list_head *pq;
 
 	if (get_user(oldfd, argp))
 		return -EFAULT;
@@ -2593,8 +2594,12 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
 	if (!fud)
 		return -EINVAL;
 
+	pq = fuse_pqueue_alloc();
+	if (!pq)
+		return -ENOMEM;
+
 	new_fud = fuse_file_to_fud(file);
-	if (!fuse_dev_install(new_fud, fud->fc))
+	if (!fuse_dev_install(new_fud, fud->fc, pq))
 		return -EINVAL;
 
 	return 0;
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 7b9822e8837b..fb4f21c871fb 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -277,7 +277,7 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
 	queue = kzalloc_obj(*queue, GFP_KERNEL_ACCOUNT);
 	if (!queue)
 		return NULL;
-	pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
+	pq = fuse_pqueue_alloc();
 	if (!pq) {
 		kfree(queue);
 		return NULL;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 92576e28f8ac..eaeda2f097dc 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1337,8 +1337,9 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 void fuse_conn_put(struct fuse_conn *fc);
 
 struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
-struct fuse_dev *fuse_dev_alloc(void);
-bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
+struct list_head *fuse_pqueue_alloc(void);
+struct fuse_dev *fuse_dev_alloc(bool alloc_pq);
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq);
 void fuse_dev_put(struct fuse_dev *fud);
 int fuse_send_init(struct fuse_mount *fm);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e42356d60f7a..f951f8d69e47 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -977,15 +977,22 @@ static void fuse_iqueue_init(struct fuse_iqueue *fiq,
 
 void fuse_pqueue_init(struct fuse_pqueue *fpq)
 {
-	unsigned int i;
-
 	spin_lock_init(&fpq->lock);
-	for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
-		INIT_LIST_HEAD(&fpq->processing[i]);
 	INIT_LIST_HEAD(&fpq->io);
 	fpq->connected = 1;
 }
 
+struct list_head *fuse_pqueue_alloc(void)
+{
+	struct list_head *pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
+
+	if (pq) {
+		for (int i = 0; i < FUSE_PQ_HASH_SIZE; i++)
+			INIT_LIST_HEAD(&pq[i]);
+	}
+	return pq;
+}
+
 void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 		    struct user_namespace *user_ns,
 		    const struct fuse_iqueue_ops *fiq_ops, void *fiq_priv)
@@ -1632,33 +1639,38 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
 	return 0;
 }
 
-struct fuse_dev *fuse_dev_alloc(void)
+struct fuse_dev *fuse_dev_alloc(bool alloc_pq)
 {
 	struct fuse_dev *fud;
-	struct list_head *pq;
+	struct list_head *pq __free(kfree) = NULL;
+
+	if (alloc_pq) {
+		pq = fuse_pqueue_alloc();
+		if (!pq)
+			return NULL;
+	}
 
 	fud = kzalloc_obj(struct fuse_dev);
 	if (!fud)
 		return NULL;
 
 	refcount_set(&fud->ref, 1);
-	pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
-	if (!pq) {
-		kfree(fud);
-		return NULL;
-	}
-
-	fud->pq.processing = pq;
+	fud->pq.processing = no_free_ptr(pq);
 	fuse_pqueue_init(&fud->pq);
 
 	return fud;
 }
 EXPORT_SYMBOL_GPL(fuse_dev_alloc);
 
-bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq)
 {
 	struct fuse_conn *old_fc;
 
+	if (!pq)
+		WARN_ON(!fud->pq.processing);
+	else if (cmpxchg(&fud->pq.processing, NULL, pq))
+		kfree(pq);
+
 	fuse_conn_get(fc);
 	spin_lock(&fc->lock);
 	/*
@@ -1686,13 +1698,12 @@ EXPORT_SYMBOL_GPL(fuse_dev_install);
 
 struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
 {
-	struct fuse_dev *fud;
+	struct fuse_dev *fud = fuse_dev_alloc(true);
 
-	fud = fuse_dev_alloc();
 	if (!fud)
 		return NULL;
 
-	fuse_dev_install(fud, fc);
+	fuse_dev_install(fud, fc, NULL);
 	return fud;
 }
 EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
@@ -1869,10 +1880,14 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
 	struct fuse_dev *fud = ctx->fud;
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	struct fuse_conn *fc = fm->fc;
+	struct list_head *pq __free(kfree) = fuse_pqueue_alloc();
 	struct inode *root;
 	struct dentry *root_dentry;
 	int err;
 
+	if (!pq)
+		return -ENOMEM;
+
 	err = -EINVAL;
 	if (sb->s_flags & SB_MANDLOCK)
 		goto err;
@@ -1944,7 +1959,7 @@ 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 (fud) {
-		if (!fuse_dev_install(fud, fc))
+		if (!fuse_dev_install(fud, fc, no_free_ptr(pq)))
 			fc->connected = 0; /* device file got closed */
 		else
 			wake_up_all(&fuse_dev_waitq);
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 12300651a0f1..cc6426992ecd 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1585,7 +1585,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
 	for (i = 0; i < fs->nvqs; i++) {
 		struct virtio_fs_vq *fsvq = &fs->vqs[i];
 
-		fsvq->fud = fuse_dev_alloc();
+		fsvq->fud = fuse_dev_alloc(true);
 		if (!fsvq->fud)
 			goto err_free_fuse_devs;
 	}
@@ -1606,7 +1606,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
 	for (i = 0; i < fs->nvqs; i++) {
 		struct virtio_fs_vq *fsvq = &fs->vqs[i];
 
-		fuse_dev_install(fsvq->fud, fc);
+		fuse_dev_install(fsvq->fud, fc, NULL);
 	}
 
 	/* Previous unmount will stop all queues. Start these again */
-- 
2.53.0

[PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option

[Miklos Szeredi]

This is not only cleaner to use in userspace (no need to sprintf the fd to
a string) but also allows userspace to detect that the devfd can be closed
after the fsconfig call.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/inode.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f951f8d69e47..5608ed5ba000 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -788,7 +788,7 @@ enum {
 
 static const struct fs_parameter_spec fuse_fs_parameters[] = {
 	fsparam_string	("source",		OPT_SOURCE),
-	fsparam_u32	("fd",			OPT_FD),
+	fsparam_fd	("fd",			OPT_FD),
 	fsparam_u32oct	("rootmode",		OPT_ROOTMODE),
 	fsparam_uid	("user_id",		OPT_USER_ID),
 	fsparam_gid	("group_id",		OPT_GROUP_ID),
@@ -800,9 +800,8 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
 	{}
 };
 
-static int fuse_opt_fd(struct fs_context *fsc, int fd)
+static int fuse_opt_fd(struct fs_context *fsc, struct file *file)
 {
-	struct file *file __free(fput) = fget(fd);
 	struct fuse_fs_context *ctx = fsc->fs_private;
 
 	if (file->f_op != &fuse_dev_operations)
@@ -859,7 +858,11 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
 		return 0;
 
 	case OPT_FD:
-		return fuse_opt_fd(fsc, result.uint_32);
+		if (param->type == fs_value_is_file)
+			return fuse_opt_fd(fsc, param->file);
+
+		struct file *file __free(fput) = fget(result.uint_32);
+		return fuse_opt_fd(fsc, file);
 
 	case OPT_ROOTMODE:
 		if (!fuse_valid_type(result.uint_32))
-- 
2.53.0

Re: [PATCH v2 0/6] fuse: fix hang with sync init

[Bernd Schubert]

On 3/12/26 20:39, Miklos Szeredi wrote:
> This is better polished, still needs more testing.
> 
> ---
> Miklos Szeredi (6):
>   fuse: create fuse_dev on /dev/fuse open instead of mount
>   fuse: add refcount to fuse_dev
>   fuse: don't require /dev/fuse fd to be kept open during mount
>   fuse: clean up device cloning
>   fuse: alloc pqueue before installing fc
>   fuse: support FSCONFIG_SET_FD for "fd" option
> 
>  fs/fuse/cuse.c       |   2 +-
>  fs/fuse/dev.c        |  89 ++++++++++------------
>  fs/fuse/dev_uring.c  |   2 +-
>  fs/fuse/fuse_dev_i.h |  30 ++++++--
>  fs/fuse/fuse_i.h     |  21 +++--
>  fs/fuse/inode.c      | 177 ++++++++++++++++++++++++++-----------------
>  fs/fuse/virtio_fs.c  |   8 +-
>  7 files changed, 181 insertions(+), 148 deletions(-)
> 

Thanks a lot Miklos! Will review it tomorrow and also get the the new
mount API done.


Thanks,
Bernd

Re: [PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option

[kernel test robot]

Hi Miklos,

kernel test robot noticed the following build errors:

[auto build test ERROR on mszeredi-fuse/for-next]
[also build test ERROR on linus/master v7.0-rc3 next-20260312]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Miklos-Szeredi/fuse-create-fuse_dev-on-dev-fuse-open-instead-of-mount/20260313-153922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
patch link:    https://lore.kernel.org/r/20260312194019.3077902-7-mszeredi%40redhat.com
patch subject: [PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260313/202603131356.P70o4OPq-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260313/202603131356.P70o4OPq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603131356.P70o4OPq-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/fuse/inode.c:916:2: error: cannot jump from switch statement to this case label
     916 |         default:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:910:2: error: cannot jump from switch statement to this case label
     910 |         case OPT_BLKSIZE:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:906:2: error: cannot jump from switch statement to this case label
     906 |         case OPT_MAX_READ:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:902:2: error: cannot jump from switch statement to this case label
     902 |         case OPT_ALLOW_OTHER:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:898:2: error: cannot jump from switch statement to this case label
     898 |         case OPT_DEFAULT_PERMISSIONS:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:886:2: error: cannot jump from switch statement to this case label
     886 |         case OPT_GROUP_ID:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:874:2: error: cannot jump from switch statement to this case label
     874 |         case OPT_USER_ID:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   fs/fuse/inode.c:867:2: error: cannot jump from switch statement to this case label
     867 |         case OPT_ROOTMODE:
         |         ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
     864 |                 struct file *file __free(fput) = fget(result.uint_32);
         |                              ^
   8 errors generated.


vim +916 fs/fuse/inode.c

510714a1f72bd3f Miklos Szeredi 2026-03-12  821  
84c215075b5723a Miklos Szeredi 2021-08-04  822  static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  823  {
c30da2e981a703c David Howells  2019-03-25  824  	struct fs_parse_result result;
84c215075b5723a Miklos Szeredi 2021-08-04  825  	struct fuse_fs_context *ctx = fsc->fs_private;
c30da2e981a703c David Howells  2019-03-25  826  	int opt;
525bd65aa759ec3 Eric Sandeen   2024-07-02  827  	kuid_t kuid;
525bd65aa759ec3 Eric Sandeen   2024-07-02  828  	kgid_t kgid;
c30da2e981a703c David Howells  2019-03-25  829  
84c215075b5723a Miklos Szeredi 2021-08-04  830  	if (fsc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  831  		/*
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  832  		 * Ignore options coming from mount(MS_REMOUNT) for backward
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  833  		 * compatibility.
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  834  		 */
84c215075b5723a Miklos Szeredi 2021-08-04  835  		if (fsc->oldapi)
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  836  			return 0;
e8b20a474cf2c42 Miklos Szeredi 2020-07-14  837  
84c215075b5723a Miklos Szeredi 2021-08-04  838  		return invalfc(fsc, "No changes allowed in reconfigure");
b330966f79fb4fd Miklos Szeredi 2020-07-14  839  	}
b330966f79fb4fd Miklos Szeredi 2020-07-14  840  
84c215075b5723a Miklos Szeredi 2021-08-04  841  	opt = fs_parse(fsc, fuse_fs_parameters, param, &result);
c30da2e981a703c David Howells  2019-03-25  842  	if (opt < 0)
c30da2e981a703c David Howells  2019-03-25  843  		return opt;
c30da2e981a703c David Howells  2019-03-25  844  
c30da2e981a703c David Howells  2019-03-25  845  	switch (opt) {
c30da2e981a703c David Howells  2019-03-25  846  	case OPT_SOURCE:
84c215075b5723a Miklos Szeredi 2021-08-04  847  		if (fsc->source)
84c215075b5723a Miklos Szeredi 2021-08-04  848  			return invalfc(fsc, "Multiple sources specified");
84c215075b5723a Miklos Szeredi 2021-08-04  849  		fsc->source = param->string;
c30da2e981a703c David Howells  2019-03-25  850  		param->string = NULL;
c30da2e981a703c David Howells  2019-03-25  851  		break;
c30da2e981a703c David Howells  2019-03-25  852  
c30da2e981a703c David Howells  2019-03-25  853  	case OPT_SUBTYPE:
c30da2e981a703c David Howells  2019-03-25  854  		if (ctx->subtype)
84c215075b5723a Miklos Szeredi 2021-08-04  855  			return invalfc(fsc, "Multiple subtypes specified");
c30da2e981a703c David Howells  2019-03-25  856  		ctx->subtype = param->string;
c30da2e981a703c David Howells  2019-03-25  857  		param->string = NULL;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  858  		return 0;
c30da2e981a703c David Howells  2019-03-25  859  
c30da2e981a703c David Howells  2019-03-25  860  	case OPT_FD:
6ad877148e563ae Miklos Szeredi 2026-03-12  861  		if (param->type == fs_value_is_file)
6ad877148e563ae Miklos Szeredi 2026-03-12  862  			return fuse_opt_fd(fsc, param->file);
6ad877148e563ae Miklos Szeredi 2026-03-12  863  
6ad877148e563ae Miklos Szeredi 2026-03-12  864  		struct file *file __free(fput) = fget(result.uint_32);
6ad877148e563ae Miklos Szeredi 2026-03-12  865  		return fuse_opt_fd(fsc, file);
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  866  
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  867  	case OPT_ROOTMODE:
c30da2e981a703c David Howells  2019-03-25  868  		if (!fuse_valid_type(result.uint_32))
84c215075b5723a Miklos Szeredi 2021-08-04  869  			return invalfc(fsc, "Invalid rootmode");
c30da2e981a703c David Howells  2019-03-25  870  		ctx->rootmode = result.uint_32;
cabdb4fa2f666fa zhengbin       2020-01-14  871  		ctx->rootmode_present = true;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  872  		break;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  873  
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  874  	case OPT_USER_ID:
eea6a8322efd398 Eric Sandeen   2024-07-02  875  		kuid = result.uid;
525bd65aa759ec3 Eric Sandeen   2024-07-02  876  		/*
525bd65aa759ec3 Eric Sandeen   2024-07-02  877  		 * The requested uid must be representable in the
525bd65aa759ec3 Eric Sandeen   2024-07-02  878  		 * filesystem's idmapping.
525bd65aa759ec3 Eric Sandeen   2024-07-02  879  		 */
525bd65aa759ec3 Eric Sandeen   2024-07-02  880  		if (!kuid_has_mapping(fsc->user_ns, kuid))
525bd65aa759ec3 Eric Sandeen   2024-07-02  881  			return invalfc(fsc, "Invalid user_id");
525bd65aa759ec3 Eric Sandeen   2024-07-02  882  		ctx->user_id = kuid;
cabdb4fa2f666fa zhengbin       2020-01-14  883  		ctx->user_id_present = true;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  884  		break;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  885  
87729a5514e855c Miklos Szeredi 2005-09-09  886  	case OPT_GROUP_ID:
eea6a8322efd398 Eric Sandeen   2024-07-02  887  		kgid = result.gid;
525bd65aa759ec3 Eric Sandeen   2024-07-02  888  		/*
525bd65aa759ec3 Eric Sandeen   2024-07-02  889  		 * The requested gid must be representable in the
525bd65aa759ec3 Eric Sandeen   2024-07-02  890  		 * filesystem's idmapping.
525bd65aa759ec3 Eric Sandeen   2024-07-02  891  		 */
525bd65aa759ec3 Eric Sandeen   2024-07-02  892  		if (!kgid_has_mapping(fsc->user_ns, kgid))
84c215075b5723a Miklos Szeredi 2021-08-04  893  			return invalfc(fsc, "Invalid group_id");
525bd65aa759ec3 Eric Sandeen   2024-07-02  894  		ctx->group_id = kgid;
cabdb4fa2f666fa zhengbin       2020-01-14  895  		ctx->group_id_present = true;
87729a5514e855c Miklos Szeredi 2005-09-09  896  		break;
87729a5514e855c Miklos Szeredi 2005-09-09  897  
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  898  	case OPT_DEFAULT_PERMISSIONS:
cabdb4fa2f666fa zhengbin       2020-01-14  899  		ctx->default_permissions = true;
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  900  		break;
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  901  
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  902  	case OPT_ALLOW_OTHER:
cabdb4fa2f666fa zhengbin       2020-01-14  903  		ctx->allow_other = true;
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  904  		break;
1e9a4ed9396e9c3 Miklos Szeredi 2005-09-09  905  
db50b96c0f28a21 Miklos Szeredi 2005-09-09  906  	case OPT_MAX_READ:
c30da2e981a703c David Howells  2019-03-25  907  		ctx->max_read = result.uint_32;
db50b96c0f28a21 Miklos Szeredi 2005-09-09  908  		break;
db50b96c0f28a21 Miklos Szeredi 2005-09-09  909  
d809161402e9f99 Miklos Szeredi 2006-12-06  910  	case OPT_BLKSIZE:
c30da2e981a703c David Howells  2019-03-25  911  		if (!ctx->is_bdev)
84c215075b5723a Miklos Szeredi 2021-08-04  912  			return invalfc(fsc, "blksize only supported for fuseblk");
c30da2e981a703c David Howells  2019-03-25  913  		ctx->blksize = result.uint_32;
d809161402e9f99 Miklos Szeredi 2006-12-06  914  		break;
d809161402e9f99 Miklos Szeredi 2006-12-06  915  
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09 @916  	default:
c30da2e981a703c David Howells  2019-03-25  917  		return -EINVAL;
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  918  	}
5a53368277efa2d Miklos Szeredi 2005-09-09  919  
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  920  	return 0;
c30da2e981a703c David Howells  2019-03-25  921  }
d8a5ba45457e4a2 Miklos Szeredi 2005-09-09  922  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/6] fuse: create fuse_dev on /dev/fuse open instead of mount

[Darrick J. Wong]

On Thu, Mar 12, 2026 at 08:39:59PM +0100, Miklos Szeredi wrote:
> Allocate struct fuse_dev when opening the device.  This means that unlike
> before, ->private_data is always set to a valid pointer.
> 
> The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
> replaced with a simple bool sync_init member.

Oh thank goodness, this became less clean in the iomap patchset when it
came to add pre-approval for iomap. :)

> If sync INIT is not set, I/O on the device returns error before mount.
> Keep this behavior by checking for the ->fc member.  If fud->fc is set, the
> mount has succeeded.  Testing this used READ_ONCE(file->private_data) and
> smp_mb() to try and provide the necessary semantics.  Switch this to
> smp_store_release() and smp_load_acquire().
> 
> Setting fud->fc is protected by fuse_mutex, this is unchanged.
> 
> Will need this later so the /dev/fuse open file reference is not held
> during FSCONFIG_CMD_CREATE.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/dev.c        | 47 +++++++++++++++++---------------------------
>  fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++--------
>  fs/fuse/fuse_i.h     |  5 ++---
>  fs/fuse/inode.c      | 35 +++++++++++----------------------
>  fs/fuse/virtio_fs.c  |  2 --
>  5 files changed, 56 insertions(+), 66 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 2c16b94357d5..4795e3d01b75 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1542,32 +1542,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
>  
>  static int fuse_dev_open(struct inode *inode, struct file *file)
>  {
> -	/*
> -	 * The fuse device's file's private_data is used to hold
> -	 * the fuse_conn(ection) when it is mounted, and is used to
> -	 * keep track of whether the file has been mounted already.
> -	 */
> -	file->private_data = NULL;
> +	struct fuse_dev *fud = fuse_dev_alloc();
> +
> +	if (!fud)
> +		return -ENOMEM;
> +
> +	file->private_data = fud;
>  	return 0;
>  }
>  
>  struct fuse_dev *fuse_get_dev(struct file *file)
>  {
> -	struct fuse_dev *fud = __fuse_get_dev(file);
> +	struct fuse_dev *fud = fuse_file_to_fud(file);
>  	int err;
>  
> -	if (likely(fud))
> -		return fud;
> -
> -	err = wait_event_interruptible(fuse_dev_waitq,
> -				       READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
> +	err = wait_event_interruptible(fuse_dev_waitq, fuse_dev_fc_get(fud));

Nit: Would it be clearer that we're waiting on a condition if this were
phrased as:

	err = wait_event_interruptible(fuse_dev_waitq,
			fuse_dev_fc_get(fud) != NULL);

?

>  	if (err)
>  		return ERR_PTR(err);
>  
> -	fud = __fuse_get_dev(file);
> -	if (!fud)
> -		return ERR_PTR(-EPERM);
> -
>  	return fud;
>  }

<snip>

> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 134bf44aff0d..522b2012cd1f 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -39,18 +39,35 @@ struct fuse_copy_state {
>  	} ring;
>  };
>  
> -#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> -#define FUSE_DEV_PTR_MASK (~1UL)
> +/*
> + * Lockless access is OK, because fud->fc is set once during mount and is valid
> + * until the file is released.
> + */
> +static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
> +{
> +	/* Pairs with smp_store_release() in fuse_dev_fc_set() */
> +	return smp_load_acquire(&fud->fc);
> +}
> +
> +static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> +{
> +	/* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> +	smp_store_release(&fud->fc, fc);
> +}
> +
> +static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
> +{
> +	return file->private_data;
> +}

/me is no expert on load-acquire/store-release, but I think this works.
We can't reorder code below the store-release, which means that if
anyone calling _fc_get actually sees a non-null fuse_conn, then the fuse
connection is fully initialized and ready to go, right?

>  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.
> -	 */
> -	struct fuse_dev *fud = READ_ONCE(file->private_data);
> +	struct fuse_dev *fud = fuse_file_to_fud(file);
> +
> +	if (!fuse_dev_fc_get(fud))
> +		return NULL;
>  
> -	return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
> +	return fud;
>  }
>  
>  struct fuse_dev *fuse_get_dev(struct file *file);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7f16049387d1..739220d96b6f 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -576,6 +576,8 @@ struct fuse_pqueue {
>   * Fuse device instance
>   */
>  struct fuse_dev {
> +	bool sync_init;

Does this need a kerneldoc comment, since the other fields have one?
Granted the only thing I can think of is

	/** Issue FUSE_INIT synchronously */


> +
>  	/** Fuse connection for this device */
>  	struct fuse_conn *fc;
>  
> @@ -622,9 +624,6 @@ struct fuse_fs_context {
>  
>  	/* DAX device, may be NULL */
>  	struct dax_device *dax_dev;
> -
> -	/* fuse_dev pointer to fill in, should contain NULL on entry */
> -	void **fudptr;

I'm glad the fudptr goes away!

If I got the smp_store_release part right then
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  };
>  
>  struct fuse_sync_bucket {
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index e57b8af06be9..53663846db36 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1637,7 +1637,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc);
>  
>  void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
>  {
> -	fud->fc = fuse_conn_get(fc);
> +	fuse_dev_fc_set(fud, fuse_conn_get(fc));
>  	spin_lock(&fc->lock);
>  	list_add_tail(&fud->entry, &fc->devices);
>  	spin_unlock(&fc->lock);
> @@ -1659,7 +1659,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
>  
>  void fuse_dev_free(struct fuse_dev *fud)
>  {
> -	struct fuse_conn *fc = fud->fc;
> +	struct fuse_conn *fc = fuse_dev_fc_get(fud);
>  
>  	if (fc) {
>  		spin_lock(&fc->lock);
> @@ -1822,7 +1822,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
>  
>  int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>  {
> -	struct fuse_dev *fud = NULL;
> +	struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
>  	struct fuse_mount *fm = get_fuse_mount_super(sb);
>  	struct fuse_conn *fc = fm->fc;
>  	struct inode *root;
> @@ -1856,18 +1856,11 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>  			goto err;
>  	}
>  
> -	if (ctx->fudptr) {
> -		err = -ENOMEM;
> -		fud = fuse_dev_alloc_install(fc);
> -		if (!fud)
> -			goto err_free_dax;
> -	}
> -
>  	fc->dev = sb->s_dev;
>  	fm->sb = sb;
>  	err = fuse_bdi_init(fc, sb);
>  	if (err)
> -		goto err_dev_free;
> +		goto err_free_dax;
>  
>  	/* Handle umasking inside the fuse code */
>  	if (sb->s_flags & SB_POSIXACL)
> @@ -1889,15 +1882,15 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>  	set_default_d_op(sb, &fuse_dentry_operations);
>  	root_dentry = d_make_root(root);
>  	if (!root_dentry)
> -		goto err_dev_free;
> +		goto err_free_dax;
>  
>  	mutex_lock(&fuse_mutex);
>  	err = -EINVAL;
> -	if (ctx->fudptr && *ctx->fudptr) {
> -		if (*ctx->fudptr == FUSE_DEV_SYNC_INIT)
> -			fc->sync_init = 1;
> -		else
> +	if (fud) {
> +		if (fuse_dev_fc_get(fud))
>  			goto err_unlock;
> +		if (fud->sync_init)
> +			fc->sync_init = 1;
>  	}
>  
>  	err = fuse_ctl_add_conn(fc);
> @@ -1906,8 +1899,8 @@ 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) {
> -		*ctx->fudptr = fud;
> +	if (fud) {
> +		fuse_dev_install(fud, fc);
>  		wake_up_all(&fuse_dev_waitq);
>  	}
>  	mutex_unlock(&fuse_mutex);
> @@ -1916,9 +1909,6 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>   err_unlock:
>  	mutex_unlock(&fuse_mutex);
>  	dput(root_dentry);
> - err_dev_free:
> -	if (fud)
> -		fuse_dev_free(fud);
>   err_free_dax:
>  	if (IS_ENABLED(CONFIG_FUSE_DAX))
>  		fuse_dax_conn_free(fc);
> @@ -1944,13 +1934,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>  	if ((ctx->file->f_op != &fuse_dev_operations) ||
>  	    (ctx->file->f_cred->user_ns != sb->s_user_ns))
>  		return -EINVAL;
> -	ctx->fudptr = &ctx->file->private_data;
>  
>  	err = fuse_fill_super_common(sb, ctx);
>  	if (err)
>  		return err;
> -	/* file->private_data shall be visible on all CPUs after this */
> -	smp_mb();
>  
>  	fm = get_fuse_mount_super(sb);
>  
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 2f7485ffac52..f685916754ad 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1590,8 +1590,6 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
>  			goto err_free_fuse_devs;
>  	}
>  
> -	/* virtiofs allocates and installs its own fuse devices */
> -	ctx->fudptr = NULL;
>  	if (ctx->dax_mode != FUSE_DAX_NEVER) {
>  		if (ctx->dax_mode == FUSE_DAX_ALWAYS && !fs->dax_dev) {
>  			err = -EINVAL;
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 2/6] fuse: add refcount to fuse_dev

[Darrick J. Wong]

On Thu, Mar 12, 2026 at 08:40:00PM +0100, Miklos Szeredi wrote:
> This will make it possible to grab the fuse_dev and subsequently release
> the file that it came from.
> 
> In the above case, fud->fc will be set to FUSE_DEV_FC_DISCONNECTED to
> indicate that this is no longer a functional device.
> 
> When trying to assign an fc to such a disconnected fuse_dev, the fc is set
> to the disconnected state.
> 
> Use atomic operations xchg() and cmpxchg() to prevent races.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/cuse.c       |  2 +-
>  fs/fuse/dev.c        |  9 +++++++--
>  fs/fuse/fuse_dev_i.h | 11 ++++-------
>  fs/fuse/fuse_i.h     |  6 ++++--
>  fs/fuse/inode.c      | 43 +++++++++++++++++++++++++++++++++++--------
>  fs/fuse/virtio_fs.c  |  2 +-
>  6 files changed, 52 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index dfcb98a654d8..174333633471 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -527,7 +527,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
>  	cc->fc.initialized = 1;
>  	rc = cuse_send_init(cc);
>  	if (rc) {
> -		fuse_dev_free(fud);
> +		fuse_dev_put(fud);
>  		return rc;
>  	}
>  	file->private_data = fud;
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 4795e3d01b75..3adf6bd38c9b 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2527,7 +2527,8 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>  int fuse_dev_release(struct inode *inode, struct file *file)
>  {
>  	struct fuse_dev *fud = fuse_file_to_fud(file);
> -	struct fuse_conn *fc = fuse_dev_fc_get(fud);
> +	/* Pairs with cmpxchg() in fuse_dev_install() */
> +	struct fuse_conn *fc = xchg(&fud->fc, FUSE_DEV_FC_DISCONNECTED);

Humm.  The previous patch introduced code that does things based on the
null-ness of the return value of fuse_dev_fc_get().  I /think/ code like
this from fuse_device_clone:

	if (fuse_dev_fc_get(new_fud))
		return -EINVAL

is ok as long as you can't clone a disconnected fuse_dev file, right?
But then there's __fuse_get_dev, which does:

	struct fuse_dev *fud = fuse_file_to_fud(file);

	if (!fuse_dev_fc_get(fud))
		return NULL;
	return fud;

at which point a disconnected fud could in theory leak out to other
parts of fuse.  Is there something to prevent other code from
dereferencing fud->fc (which is 0x1) and blowing up?

I guess the answer might be that you've closed /dev/fuse so there aren't
going to be more requests coming in from userspace and all we're doing
is tearing down the fud?  But why not set fud->fc to NULL on
disconnection?

>  	if (fc) {

BTW, is there a chance that we can race here?  I think the answer is
'no' because only one thread can ->release the fuse device file, right?

>  		struct fuse_pqueue *fpq = &fud->pq;
> @@ -2547,8 +2548,12 @@ int fuse_dev_release(struct inode *inode, struct file *file)
>  			WARN_ON(fc->iq.fasync != NULL);
>  			fuse_abort_conn(fc);
>  		}
> +		spin_lock(&fc->lock);
> +		list_del(&fud->entry);
> +		spin_unlock(&fc->lock);
> +		fuse_conn_put(fc);
>  	}
> -	fuse_dev_free(fud);
> +	fuse_dev_put(fud);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_release);
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 522b2012cd1f..9eae14ace73c 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -39,22 +39,19 @@ struct fuse_copy_state {
>  	} ring;
>  };
>  
> +/* fud->fc gets assigned to this value when /dev/fuse is closed */
> +#define FUSE_DEV_FC_DISCONNECTED ((struct fuse_conn *) 1)
> +
>  /*
>   * Lockless access is OK, because fud->fc is set once during mount and is valid
>   * until the file is released.
>   */
>  static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
>  {
> -	/* Pairs with smp_store_release() in fuse_dev_fc_set() */
> +	/* Pairs with xchg() in fuse_dev_install() */
>  	return smp_load_acquire(&fud->fc);
>  }
>  
> -static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> -{
> -	/* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> -	smp_store_release(&fud->fc, fc);
> -}
> -
>  static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
>  {
>  	return file->private_data;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 739220d96b6f..66ec92f06497 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -576,6 +576,8 @@ struct fuse_pqueue {
>   * Fuse device instance
>   */
>  struct fuse_dev {
> +	refcount_t ref;
> +
>  	bool sync_init;
>  
>  	/** Fuse connection for this device */
> @@ -1341,8 +1343,8 @@ void fuse_conn_put(struct fuse_conn *fc);
>  
>  struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
>  struct fuse_dev *fuse_dev_alloc(void);
> -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> -void fuse_dev_free(struct fuse_dev *fud);
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> +void fuse_dev_put(struct fuse_dev *fud);
>  int fuse_send_init(struct fuse_mount *fm);
>  
>  /**
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 53663846db36..ba845092e7f2 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1622,6 +1622,7 @@ struct fuse_dev *fuse_dev_alloc(void)
>  	if (!fud)
>  		return NULL;
>  
> +	refcount_set(&fud->ref, 1);
>  	pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
>  	if (!pq) {
>  		kfree(fud);
> @@ -1635,12 +1636,32 @@ struct fuse_dev *fuse_dev_alloc(void)
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_alloc);
>  
> -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
>  {
> -	fuse_dev_fc_set(fud, fuse_conn_get(fc));
> +	struct fuse_conn *old_fc;
> +
> +	fuse_conn_get(fc);
>  	spin_lock(&fc->lock);
> +	/*
> +	 * Pairs with:
> +	 *  - xchg() in fuse_dev_release()
> +	 *  - smp_load_acquire() in fuse_dev_fc_get()
> +	 */
> +	old_fc = cmpxchg(&fud->fc, NULL, fc);
> +	if (old_fc) {
> +		/*
> +		 * failed to set fud->fc because
> +		 *  - it was already set to a different fc
> +		 *  - it was set to disconneted
> +		 */
> +		spin_unlock(&fc->lock);
> +		fuse_conn_put(fc);
> +		return false;
> +	}
>  	list_add_tail(&fud->entry, &fc->devices);
>  	spin_unlock(&fc->lock);
> +
> +	return true;
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_install);
>  
> @@ -1657,11 +1678,15 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
>  
> -void fuse_dev_free(struct fuse_dev *fud)
> +void fuse_dev_put(struct fuse_dev *fud)
>  {
> -	struct fuse_conn *fc = fuse_dev_fc_get(fud);
> +	struct fuse_conn *fc;
>  
> -	if (fc) {
> +	if (!refcount_dec_and_test(&fud->ref))
> +		return;
> +
> +	fc = fuse_dev_fc_get(fud);
> +	if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {

AFAICT this is the only place in the whole patchset that even looks for
_DISCONNECTED, which makes me wonder even more what distinguishes it
from plain old NULL?

--D

>  		spin_lock(&fc->lock);
>  		list_del(&fud->entry);
>  		spin_unlock(&fc->lock);
> @@ -1671,7 +1696,7 @@ void fuse_dev_free(struct fuse_dev *fud)
>  	kfree(fud->pq.processing);
>  	kfree(fud);
>  }
> -EXPORT_SYMBOL_GPL(fuse_dev_free);
> +EXPORT_SYMBOL_GPL(fuse_dev_put);
>  
>  static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
>  				      const struct fuse_inode *fi)
> @@ -1900,8 +1925,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 (fud) {
> -		fuse_dev_install(fud, fc);
> -		wake_up_all(&fuse_dev_waitq);
> +		if (!fuse_dev_install(fud, fc))
> +			fc->connected = 0; /* device file got closed */
> +		else
> +			wake_up_all(&fuse_dev_waitq);
>  	}
>  	mutex_unlock(&fuse_mutex);
>  	return 0;
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index f685916754ad..12300651a0f1 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -486,7 +486,7 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
>  		if (!fsvq->fud)
>  			continue;
>  
> -		fuse_dev_free(fsvq->fud);
> +		fuse_dev_put(fsvq->fud);
>  		fsvq->fud = NULL;
>  	}
>  }
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Darrick J. Wong]

On Thu, Mar 12, 2026 at 08:40:01PM +0100, Miklos Szeredi wrote:
> With the new mount API the sequence of syscalls would be:
> 
>         fs_fd = fsopen("fuse", 0);
> 	snprintf(opt, sizeof(opt), "%i", devfd);
> 	fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", opt, 0);
> 	/* ... */
> 	fsconfig(fs_fd, FSCONFIG_CMD_CREATE, 0, 0, 0);
> 
> Current mount code just stores the value of devfd in the fs_context and
> uses it in during FSCONFIG_CMD_CREATE.
> 
> This is not very elegant, but there's a bigger problem: when sync init is
> used and the server exits for some reason (error, crash) while processing
> FUSE_INIT, the filesystem creation will hang.

Hrmm, is this https://github.com/libfuse/libfuse/pull/1367 ?

Which syscall causes the synchronous FUSE_INIT to be sent?  Is it
FSCONFIG_CMD_CREATE?

> The reason is that while all other threads will exit, the mounting
> thread (or process) will keep the device fd open, which will prevent
> an abort from happening.

So I guess what's happening here is that the main thread calls
FSCONFIG_CMD_CREATE, which sends the synchronous FUSE_INIT to the worker
pool.  One of the worker threads starts working on the FUSE_INIT reply
and crashes.  All the *workers* terminate and close the fuse dev fd,
leaving just the main thread.

AFAICT, the main thread is stuck in fsconfig() here:

	/* Any signal may interrupt this */
		err = wait_event_interruptible(req->waitq,
				test_bit(FR_FINISHED, &req->flags))

so there's still an open ref to the fuse dev fd, which prevents anyone
from aborting the FUSE_INIT request so that FR_FINISH gets set?  And now
we're just hosed?  Wouldn't the SIGCHLD interrupt the wait?  Or are we
stuck someplace else?

> This is a regression from the async mount case, where the mount was done
> first, and the FUSE_INIT processing afterwards, in which case there's no
> such recursive syscall keeping the fd open.

Is this hang possible if you're using mount(2) with synchronous
FUSE_INIT?

> The solution is twofold:
> 
>  a) use unshare(CLONE_FILES) in the mounting thread

Is this after starting up the worker threads?  I guess that means the
worker threads retain their fuse dev fds even though...

>     and close the device fd
>     after fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", ...)

...the main thread closes the fuse dev fd before FSCONFIG_CMD_CREATE.
What if that main thread needs to use the fuse dev fd after this point?
Or, what if userspace doesn't cooperate and unshare()/close()?  Can this
hang be broken by kill -9, at least?

>  b) only reference the fuse_dev from fs_context not the device file itself

Can you set an abort timeout on the FUSE_INIT request?

Perhaps my broader question is, what /does/ happen if a fuse server
thread starts processing a request and crashes out before replying?  I
guess that means the request is never completed, but in the case of
!(synchronous FUSE_INIT) we'd just see the whole server terminate, which
would then release the fuse dev fd?

Sorry this isn't a review so much as me asking a lot of annoying
questions. :/

--D

> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/fuse_i.h |  4 +---
>  fs/fuse/inode.c  | 54 +++++++++++++++++++++++++++---------------------
>  2 files changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 66ec92f06497..b77b384b0385 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -603,13 +603,11 @@ static inline bool fuse_is_inode_dax_mode(enum fuse_dax_mode mode)
>  }
>  
>  struct fuse_fs_context {
> -	int fd;
> -	struct file *file;
> +	struct fuse_dev *fud;
>  	unsigned int rootmode;
>  	kuid_t user_id;
>  	kgid_t group_id;
>  	bool is_bdev:1;
> -	bool fd_present:1;
>  	bool rootmode_present:1;
>  	bool user_id_present:1;
>  	bool group_id_present:1;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index ba845092e7f2..45abcfec03a4 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -800,6 +800,26 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
>  	{}
>  };
>  
> +static int fuse_opt_fd(struct fs_context *fsc, int fd)
> +{
> +	struct file *file __free(fput) = fget(fd);
> +	struct fuse_fs_context *ctx = fsc->fs_private;
> +
> +	if (file->f_op != &fuse_dev_operations)
> +		return invalfc(fsc, "fd is not a fuse device");
> +	/*
> +	 * Require mount to happen from the same user namespace which
> +	 * opened /dev/fuse to prevent potential attacks.
> +	 */
> +	if (file->f_cred->user_ns != fsc->user_ns)
> +		return invalfc(fsc, "wrong user namespace for fuse device");
> +
> +	ctx->fud = file->private_data;
> +	refcount_inc(&ctx->fud->ref);
> +
> +	return 0;
> +}
> +
>  static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
>  {
>  	struct fs_parse_result result;
> @@ -839,9 +859,7 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
>  		return 0;
>  
>  	case OPT_FD:
> -		ctx->fd = result.uint_32;
> -		ctx->fd_present = true;
> -		break;
> +		return fuse_opt_fd(fsc, result.uint_32);
>  
>  	case OPT_ROOTMODE:
>  		if (!fuse_valid_type(result.uint_32))
> @@ -904,6 +922,8 @@ static void fuse_free_fsc(struct fs_context *fsc)
>  	struct fuse_fs_context *ctx = fsc->fs_private;
>  
>  	if (ctx) {
> +		if (ctx->fud)
> +			fuse_dev_put(ctx->fud);
>  		kfree(ctx->subtype);
>  		kfree(ctx);
>  	}
> @@ -1847,7 +1867,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
>  
>  int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>  {
> -	struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
> +	struct fuse_dev *fud = ctx->fud;
>  	struct fuse_mount *fm = get_fuse_mount_super(sb);
>  	struct fuse_conn *fc = fm->fc;
>  	struct inode *root;
> @@ -1950,18 +1970,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
>  	struct fuse_mount *fm;
>  	int err;
>  
> -	if (!ctx->file || !ctx->rootmode_present ||
> +	if (!ctx->fud || !ctx->rootmode_present ||
>  	    !ctx->user_id_present || !ctx->group_id_present)
>  		return -EINVAL;
>  
> -	/*
> -	 * Require mount to happen from the same user namespace which
> -	 * opened /dev/fuse to prevent potential attacks.
> -	 */
> -	if ((ctx->file->f_op != &fuse_dev_operations) ||
> -	    (ctx->file->f_cred->user_ns != sb->s_user_ns))
> -		return -EINVAL;
> -
>  	err = fuse_fill_super_common(sb, ctx);
>  	if (err)
>  		return err;
> @@ -1989,8 +2001,7 @@ static int fuse_test_super(struct super_block *sb, struct fs_context *fsc)
>  static int fuse_get_tree(struct fs_context *fsc)
>  {
>  	struct fuse_fs_context *ctx = fsc->fs_private;
> -	struct fuse_dev *fud;
> -	struct fuse_conn *fc;
> +	struct fuse_conn *fc, *key;
>  	struct fuse_mount *fm;
>  	struct super_block *sb;
>  	int err;
> @@ -2010,9 +2021,6 @@ static int fuse_get_tree(struct fs_context *fsc)
>  
>  	fsc->s_fs_info = fm;
>  
> -	if (ctx->fd_present)
> -		ctx->file = fget(ctx->fd);
> -
>  	if (IS_ENABLED(CONFIG_BLOCK) && ctx->is_bdev) {
>  		err = get_tree_bdev(fsc, fuse_fill_super);
>  		goto out;
> @@ -2022,16 +2030,16 @@ static int fuse_get_tree(struct fs_context *fsc)
>  	 * (found by device name), normal fuse mounts can't
>  	 */
>  	err = -EINVAL;
> -	if (!ctx->file)
> +	if (!ctx->fud)
>  		goto out;
>  
>  	/*
>  	 * Allow creating a fuse mount with an already initialized fuse
>  	 * connection
>  	 */
> -	fud = __fuse_get_dev(ctx->file);
> -	if (ctx->file->f_op == &fuse_dev_operations && fud) {
> -		fsc->sget_key = fud->fc;
> +	key = fuse_dev_fc_get(ctx->fud);
> +	if (key) {
> +		fsc->sget_key = key;
>  		sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
>  		err = PTR_ERR_OR_ZERO(sb);
>  		if (!IS_ERR(sb))
> @@ -2042,8 +2050,6 @@ static int fuse_get_tree(struct fs_context *fsc)
>  out:
>  	if (fsc->s_fs_info)
>  		fuse_mount_destroy(fm);
> -	if (ctx->file)
> -		fput(ctx->file);
>  	return err;
>  }
>  
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 4/6] fuse: clean up device cloning

[Darrick J. Wong]

On Thu, Mar 12, 2026 at 08:40:02PM +0100, Miklos Szeredi wrote:
>  - fuse_mutex is not needed for device cloning, because fuse_dev_install()
>    uses cmpxcg() to set fud->fc, which prevents races between clone/mount
>    or clone/clone.  This makes the logic simpler
> 
>  - Drop fc->dev_count.  This is only used to check in release if the device
>    is the last clone, but checking list_empty(&fc->devices) is equivalent
>    after removing the released device from the list.  Removing the fuse_dev
>    before calling fuse_abort_conn() is okay, since the processing and io
>    lists are now empty for this device.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/dev.c    | 44 ++++++++++++++++----------------------------
>  fs/fuse/fuse_i.h |  3 ---
>  fs/fuse/inode.c  |  1 -
>  3 files changed, 16 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 3adf6bd38c9b..18cc844cf290 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2543,14 +2543,15 @@ int fuse_dev_release(struct inode *inode, struct file *file)
>  
>  		fuse_dev_end_requests(&to_end);
>  
> +		spin_lock(&fc->lock);
> +		list_del(&fud->entry);
> +		spin_unlock(&fc->lock);
> +
>  		/* Are we the last open device? */
> -		if (atomic_dec_and_test(&fc->dev_count)) {
> +		if (list_empty(&fc->devices)) {

If you have two threads closing their cloned fuse dev fds, can they both
see an empty list here and abort the connection?  I think they can, but
it's benign because the abort takes its own spinlock.

Aside from that question, this looks ok to me.

--D

>  			WARN_ON(fc->iq.fasync != NULL);
>  			fuse_abort_conn(fc);
>  		}
> -		spin_lock(&fc->lock);
> -		list_del(&fud->entry);
> -		spin_unlock(&fc->lock);
>  		fuse_conn_put(fc);
>  	}
>  	fuse_dev_put(fud);
> @@ -2569,24 +2570,10 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
>  	return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
>  }
>  
> -static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> -{
> -	struct fuse_dev *new_fud = fuse_file_to_fud(new);
> -
> -	if (fuse_dev_fc_get(new_fud))
> -		return -EINVAL;
> -
> -	fuse_dev_install(new_fud, fc);
> -	atomic_inc(&fc->dev_count);
> -
> -	return 0;
> -}
> -
>  static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
>  {
> -	int res;
>  	int oldfd;
> -	struct fuse_dev *fud = NULL;
> +	struct fuse_dev *fud, *new_fud;
>  
>  	if (get_user(oldfd, argp))
>  		return -EFAULT;
> @@ -2599,17 +2586,18 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
>  	 * Check against file->f_op because CUSE
>  	 * uses the same ioctl handler.
>  	 */
> -	if (fd_file(f)->f_op == file->f_op)
> -		fud = __fuse_get_dev(fd_file(f));
> +	if (fd_file(f)->f_op != file->f_op)
> +		return -EINVAL;
>  
> -	res = -EINVAL;
> -	if (fud) {
> -		mutex_lock(&fuse_mutex);
> -		res = fuse_device_clone(fud->fc, file);
> -		mutex_unlock(&fuse_mutex);
> -	}
> +	fud = __fuse_get_dev(fd_file(f));
> +	if (!fud)
> +		return -EINVAL;
>  
> -	return res;
> +	new_fud = fuse_file_to_fud(file);
> +	if (!fuse_dev_install(new_fud, fud->fc))
> +		return -EINVAL;
> +
> +	return 0;
>  }
>  
>  static long fuse_dev_ioctl_backing_open(struct file *file,
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index b77b384b0385..92576e28f8ac 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -647,9 +647,6 @@ struct fuse_conn {
>  	/** Refcount */
>  	refcount_t count;
>  
> -	/** Number of fuse_dev's */
> -	atomic_t dev_count;
> -
>  	/** Current epoch for up-to-date dentries */
>  	atomic_t epoch;
>  
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 45abcfec03a4..e42356d60f7a 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -995,7 +995,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>  	spin_lock_init(&fc->bg_lock);
>  	init_rwsem(&fc->killsb);
>  	refcount_set(&fc->count, 1);
> -	atomic_set(&fc->dev_count, 1);
>  	atomic_set(&fc->epoch, 1);
>  	INIT_WORK(&fc->epoch_work, fuse_epoch_work);
>  	init_waitqueue_head(&fc->blocked_waitq);
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option

[Darrick J. Wong]

On Thu, Mar 12, 2026 at 08:40:04PM +0100, Miklos Szeredi wrote:
> This is not only cleaner to use in userspace (no need to sprintf the fd to
> a string) but also allows userspace to detect that the devfd can be closed
> after the fsconfig call.

Heh, I was just about to reply to the previous revision with "hey, have
you thought about converting to fsparam_fd?" :)

> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>

Looks fine to me -- the fuse dev file points to a fud, and the the fud
won't outlast the fd so the fget/fput look right to me.

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/fuse/inode.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index f951f8d69e47..5608ed5ba000 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -788,7 +788,7 @@ enum {
>  
>  static const struct fs_parameter_spec fuse_fs_parameters[] = {
>  	fsparam_string	("source",		OPT_SOURCE),
> -	fsparam_u32	("fd",			OPT_FD),
> +	fsparam_fd	("fd",			OPT_FD),
>  	fsparam_u32oct	("rootmode",		OPT_ROOTMODE),
>  	fsparam_uid	("user_id",		OPT_USER_ID),
>  	fsparam_gid	("group_id",		OPT_GROUP_ID),
> @@ -800,9 +800,8 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
>  	{}
>  };
>  
> -static int fuse_opt_fd(struct fs_context *fsc, int fd)
> +static int fuse_opt_fd(struct fs_context *fsc, struct file *file)
>  {
> -	struct file *file __free(fput) = fget(fd);
>  	struct fuse_fs_context *ctx = fsc->fs_private;
>  
>  	if (file->f_op != &fuse_dev_operations)
> @@ -859,7 +858,11 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
>  		return 0;
>  
>  	case OPT_FD:
> -		return fuse_opt_fd(fsc, result.uint_32);
> +		if (param->type == fs_value_is_file)
> +			return fuse_opt_fd(fsc, param->file);
> +
> +		struct file *file __free(fput) = fget(result.uint_32);
> +		return fuse_opt_fd(fsc, file);
>  
>  	case OPT_ROOTMODE:
>  		if (!fuse_valid_type(result.uint_32))
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option

[kernel test robot]

Hi Miklos,

kernel test robot noticed the following build errors:

[auto build test ERROR on mszeredi-fuse/for-next]
[also build test ERROR on linus/master v7.0-rc3 next-20260311]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Miklos-Szeredi/fuse-create-fuse_dev-on-dev-fuse-open-instead-of-mount/20260313-153922
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
patch link:    https://lore.kernel.org/r/20260312194019.3077902-7-mszeredi%40redhat.com
patch subject: [PATCH v2 6/6] fuse: support FSCONFIG_SET_FD for "fd" option
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20260315/202603150134.fG997K63-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260315/202603150134.fG997K63-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603150134.fG997K63-lkp@intel.com/

All errors (new ones prefixed by >>):

>> fs/fuse/inode.c:916:2: error: cannot jump from switch statement to this case label
           default:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:910:2: error: cannot jump from switch statement to this case label
           case OPT_BLKSIZE:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:906:2: error: cannot jump from switch statement to this case label
           case OPT_MAX_READ:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:902:2: error: cannot jump from switch statement to this case label
           case OPT_ALLOW_OTHER:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:898:2: error: cannot jump from switch statement to this case label
           case OPT_DEFAULT_PERMISSIONS:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:886:2: error: cannot jump from switch statement to this case label
           case OPT_GROUP_ID:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:874:2: error: cannot jump from switch statement to this case label
           case OPT_USER_ID:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   fs/fuse/inode.c:867:2: error: cannot jump from switch statement to this case label
           case OPT_ROOTMODE:
           ^
   fs/fuse/inode.c:864:16: note: jump bypasses initialization of variable with __attribute__((cleanup))
                   struct file *file __free(fput) = fget(result.uint_32);
                                ^
   8 errors generated.


vim +916 fs/fuse/inode.c

510714a1f72bd3 Miklos Szeredi 2026-03-12  821  
84c215075b5723 Miklos Szeredi 2021-08-04  822  static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
d8a5ba45457e4a Miklos Szeredi 2005-09-09  823  {
c30da2e981a703 David Howells  2019-03-25  824  	struct fs_parse_result result;
84c215075b5723 Miklos Szeredi 2021-08-04  825  	struct fuse_fs_context *ctx = fsc->fs_private;
c30da2e981a703 David Howells  2019-03-25  826  	int opt;
525bd65aa759ec Eric Sandeen   2024-07-02  827  	kuid_t kuid;
525bd65aa759ec Eric Sandeen   2024-07-02  828  	kgid_t kgid;
c30da2e981a703 David Howells  2019-03-25  829  
84c215075b5723 Miklos Szeredi 2021-08-04  830  	if (fsc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  831  		/*
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  832  		 * Ignore options coming from mount(MS_REMOUNT) for backward
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  833  		 * compatibility.
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  834  		 */
84c215075b5723 Miklos Szeredi 2021-08-04  835  		if (fsc->oldapi)
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  836  			return 0;
e8b20a474cf2c4 Miklos Szeredi 2020-07-14  837  
84c215075b5723 Miklos Szeredi 2021-08-04  838  		return invalfc(fsc, "No changes allowed in reconfigure");
b330966f79fb4f Miklos Szeredi 2020-07-14  839  	}
b330966f79fb4f Miklos Szeredi 2020-07-14  840  
84c215075b5723 Miklos Szeredi 2021-08-04  841  	opt = fs_parse(fsc, fuse_fs_parameters, param, &result);
c30da2e981a703 David Howells  2019-03-25  842  	if (opt < 0)
c30da2e981a703 David Howells  2019-03-25  843  		return opt;
c30da2e981a703 David Howells  2019-03-25  844  
c30da2e981a703 David Howells  2019-03-25  845  	switch (opt) {
c30da2e981a703 David Howells  2019-03-25  846  	case OPT_SOURCE:
84c215075b5723 Miklos Szeredi 2021-08-04  847  		if (fsc->source)
84c215075b5723 Miklos Szeredi 2021-08-04  848  			return invalfc(fsc, "Multiple sources specified");
84c215075b5723 Miklos Szeredi 2021-08-04  849  		fsc->source = param->string;
c30da2e981a703 David Howells  2019-03-25  850  		param->string = NULL;
c30da2e981a703 David Howells  2019-03-25  851  		break;
c30da2e981a703 David Howells  2019-03-25  852  
c30da2e981a703 David Howells  2019-03-25  853  	case OPT_SUBTYPE:
c30da2e981a703 David Howells  2019-03-25  854  		if (ctx->subtype)
84c215075b5723 Miklos Szeredi 2021-08-04  855  			return invalfc(fsc, "Multiple subtypes specified");
c30da2e981a703 David Howells  2019-03-25  856  		ctx->subtype = param->string;
c30da2e981a703 David Howells  2019-03-25  857  		param->string = NULL;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  858  		return 0;
c30da2e981a703 David Howells  2019-03-25  859  
c30da2e981a703 David Howells  2019-03-25  860  	case OPT_FD:
6ad877148e563a Miklos Szeredi 2026-03-12  861  		if (param->type == fs_value_is_file)
6ad877148e563a Miklos Szeredi 2026-03-12  862  			return fuse_opt_fd(fsc, param->file);
6ad877148e563a Miklos Szeredi 2026-03-12  863  
6ad877148e563a Miklos Szeredi 2026-03-12  864  		struct file *file __free(fput) = fget(result.uint_32);
6ad877148e563a Miklos Szeredi 2026-03-12  865  		return fuse_opt_fd(fsc, file);
d8a5ba45457e4a Miklos Szeredi 2005-09-09  866  
d8a5ba45457e4a Miklos Szeredi 2005-09-09  867  	case OPT_ROOTMODE:
c30da2e981a703 David Howells  2019-03-25  868  		if (!fuse_valid_type(result.uint_32))
84c215075b5723 Miklos Szeredi 2021-08-04  869  			return invalfc(fsc, "Invalid rootmode");
c30da2e981a703 David Howells  2019-03-25  870  		ctx->rootmode = result.uint_32;
cabdb4fa2f666f zhengbin       2020-01-14  871  		ctx->rootmode_present = true;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  872  		break;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  873  
d8a5ba45457e4a Miklos Szeredi 2005-09-09  874  	case OPT_USER_ID:
eea6a8322efd39 Eric Sandeen   2024-07-02  875  		kuid = result.uid;
525bd65aa759ec Eric Sandeen   2024-07-02  876  		/*
525bd65aa759ec Eric Sandeen   2024-07-02  877  		 * The requested uid must be representable in the
525bd65aa759ec Eric Sandeen   2024-07-02  878  		 * filesystem's idmapping.
525bd65aa759ec Eric Sandeen   2024-07-02  879  		 */
525bd65aa759ec Eric Sandeen   2024-07-02  880  		if (!kuid_has_mapping(fsc->user_ns, kuid))
525bd65aa759ec Eric Sandeen   2024-07-02  881  			return invalfc(fsc, "Invalid user_id");
525bd65aa759ec Eric Sandeen   2024-07-02  882  		ctx->user_id = kuid;
cabdb4fa2f666f zhengbin       2020-01-14  883  		ctx->user_id_present = true;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  884  		break;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  885  
87729a5514e855 Miklos Szeredi 2005-09-09  886  	case OPT_GROUP_ID:
eea6a8322efd39 Eric Sandeen   2024-07-02  887  		kgid = result.gid;
525bd65aa759ec Eric Sandeen   2024-07-02  888  		/*
525bd65aa759ec Eric Sandeen   2024-07-02  889  		 * The requested gid must be representable in the
525bd65aa759ec Eric Sandeen   2024-07-02  890  		 * filesystem's idmapping.
525bd65aa759ec Eric Sandeen   2024-07-02  891  		 */
525bd65aa759ec Eric Sandeen   2024-07-02  892  		if (!kgid_has_mapping(fsc->user_ns, kgid))
84c215075b5723 Miklos Szeredi 2021-08-04  893  			return invalfc(fsc, "Invalid group_id");
525bd65aa759ec Eric Sandeen   2024-07-02  894  		ctx->group_id = kgid;
cabdb4fa2f666f zhengbin       2020-01-14  895  		ctx->group_id_present = true;
87729a5514e855 Miklos Szeredi 2005-09-09  896  		break;
87729a5514e855 Miklos Szeredi 2005-09-09  897  
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  898  	case OPT_DEFAULT_PERMISSIONS:
cabdb4fa2f666f zhengbin       2020-01-14  899  		ctx->default_permissions = true;
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  900  		break;
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  901  
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  902  	case OPT_ALLOW_OTHER:
cabdb4fa2f666f zhengbin       2020-01-14  903  		ctx->allow_other = true;
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  904  		break;
1e9a4ed9396e9c Miklos Szeredi 2005-09-09  905  
db50b96c0f28a2 Miklos Szeredi 2005-09-09  906  	case OPT_MAX_READ:
c30da2e981a703 David Howells  2019-03-25  907  		ctx->max_read = result.uint_32;
db50b96c0f28a2 Miklos Szeredi 2005-09-09  908  		break;
db50b96c0f28a2 Miklos Szeredi 2005-09-09  909  
d809161402e9f9 Miklos Szeredi 2006-12-06  910  	case OPT_BLKSIZE:
c30da2e981a703 David Howells  2019-03-25  911  		if (!ctx->is_bdev)
84c215075b5723 Miklos Szeredi 2021-08-04  912  			return invalfc(fsc, "blksize only supported for fuseblk");
c30da2e981a703 David Howells  2019-03-25  913  		ctx->blksize = result.uint_32;
d809161402e9f9 Miklos Szeredi 2006-12-06  914  		break;
d809161402e9f9 Miklos Szeredi 2006-12-06  915  
d8a5ba45457e4a Miklos Szeredi 2005-09-09 @916  	default:
c30da2e981a703 David Howells  2019-03-25  917  		return -EINVAL;
d8a5ba45457e4a Miklos Szeredi 2005-09-09  918  	}
5a53368277efa2 Miklos Szeredi 2005-09-09  919  
d8a5ba45457e4a Miklos Szeredi 2005-09-09  920  	return 0;
c30da2e981a703 David Howells  2019-03-25  921  }
d8a5ba45457e4a Miklos Szeredi 2005-09-09  922  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/6] fuse: create fuse_dev on /dev/fuse open instead of mount

[Miklos Szeredi]

On Fri, 13 Mar 2026 at 23:05, Darrick J. Wong <djwong@kernel.org> wrote:

> > +/*
> > + * Lockless access is OK, because fud->fc is set once during mount and is valid
> > + * until the file is released.
> > + */
> > +static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
> > +{
> > +     /* Pairs with smp_store_release() in fuse_dev_fc_set() */
> > +     return smp_load_acquire(&fud->fc);
> > +}
> > +
> > +static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> > +{
> > +     /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> > +     smp_store_release(&fud->fc, fc);
> > +}
> > +
> > +static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
> > +{
> > +     return file->private_data;
> > +}
>
> /me is no expert on load-acquire/store-release, but I think this works.
> We can't reorder code below the store-release, which means that if
> anyone calling _fc_get actually sees a non-null fuse_conn, then the fuse
> connection is fully initialized and ready to go, right?

I can't claim to be an expert either, but yes, your description is
exactly how I understand.

Nits fixed.

Thanks,
Miklos

Re: [PATCH v2 2/6] fuse: add refcount to fuse_dev

[Miklos Szeredi]

On Fri, 13 Mar 2026 at 23:28, Darrick J. Wong <djwong@kernel.org> wrote:

> Humm.  The previous patch introduced code that does things based on the
> null-ness of the return value of fuse_dev_fc_get().  I /think/ code like
> this from fuse_device_clone:
>
>         if (fuse_dev_fc_get(new_fud))
>                 return -EINVAL
>
> is ok as long as you can't clone a disconnected fuse_dev file, right?
> But then there's __fuse_get_dev, which does:
>
>         struct fuse_dev *fud = fuse_file_to_fud(file);
>
>         if (!fuse_dev_fc_get(fud))
>                 return NULL;
>         return fud;
>
> at which point a disconnected fud could in theory leak out to other
> parts of fuse.  Is there something to prevent other code from
> dereferencing fud->fc (which is 0x1) and blowing up?

These take a struct file so ->release() must not have been called.

> I guess the answer might be that you've closed /dev/fuse so there aren't
> going to be more requests coming in from userspace and all we're doing
> is tearing down the fud?  But why not set fud->fc to NULL on
> disconnection?

close(devfd) being called before or concurrently with fsconfig(fs_fd,
FSCONFIG_CMD_CREATE, ...).   This is not a practical scenario, no sane
fuse server would do this, but it still needs to be handled.

I'd love to simplify this, but it's either the xchg magic, or a new
spin lock and a status flag.  The latter is arguably less subtle, so
I'm amenable in that direction.

> >       if (fc) {
>
> BTW, is there a chance that we can race here?  I think the answer is
> 'no' because only one thread can ->release the fuse device file, right?

Yes, ->release() can only be called once, so we are sure that fud->fc
is either a fully set up connection or NULL.

> > -void fuse_dev_free(struct fuse_dev *fud)
> > +void fuse_dev_put(struct fuse_dev *fud)
> >  {
> > -     struct fuse_conn *fc = fuse_dev_fc_get(fud);
> > +     struct fuse_conn *fc;
> >
> > -     if (fc) {
> > +     if (!refcount_dec_and_test(&fud->ref))
> > +             return;
> > +
> > +     fc = fuse_dev_fc_get(fud);
> > +     if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {
>
> AFAICT this is the only place in the whole patchset that even looks for
> _DISCONNECTED, which makes me wonder even more what distinguishes it
> from plain old NULL?

fuse_dev_install() is where it matters.

Here we are testing FUSE_DEV_FC_DISCONNECTED, because virtiofs will
get here without fuse_dev_release() being called previously, so need
to clean up fuse_conn.

Added a comments.

Thanks,
Miklos

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Miklos Szeredi]

On Sat, 14 Mar 2026 at 00:09, Darrick J. Wong <djwong@kernel.org> wrote:

> Hrmm, is this https://github.com/libfuse/libfuse/pull/1367 ?

Right.

> Which syscall causes the synchronous FUSE_INIT to be sent?  Is it
> FSCONFIG_CMD_CREATE?

Yes.

> > The reason is that while all other threads will exit, the mounting
> > thread (or process) will keep the device fd open, which will prevent
> > an abort from happening.
>
> So I guess what's happening here is that the main thread calls
> FSCONFIG_CMD_CREATE, which sends the synchronous FUSE_INIT to the worker
> pool.  One of the worker threads starts working on the FUSE_INIT reply
> and crashes.  All the *workers* terminate and close the fuse dev fd,
> leaving just the main thread.
>
> AFAICT, the main thread is stuck in fsconfig() here:
>
>         /* Any signal may interrupt this */
>                 err = wait_event_interruptible(req->waitq,
>                                 test_bit(FR_FINISHED, &req->flags))
>
> so there's still an open ref to the fuse dev fd, which prevents anyone
> from aborting the FUSE_INIT request so that FR_FINISH gets set?

Yes.

>  And now
> we're just hosed?  Wouldn't the SIGCHLD interrupt the wait?  Or are we
> stuck someplace else?

The FUSE_INIT request is in FR_SENT state, and fuse doesn't allow
interrupting such requests without the cooperation of the server.

We could special case FUSE_INIT (and probably a number of other
request types) that are safe to kill without the server's consent.
Will look into this.

>
> > This is a regression from the async mount case, where the mount was done
> > first, and the FUSE_INIT processing afterwards, in which case there's no
> > such recursive syscall keeping the fd open.
>
> Is this hang possible if you're using mount(2) with synchronous
> FUSE_INIT?

Yes.

> > The solution is twofold:
> >
> >  a) use unshare(CLONE_FILES) in the mounting thread
>
> Is this after starting up the worker threads?  I guess that means the
> worker threads retain their fuse dev fds even though...
>
> >     and close the device fd
> >     after fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", ...)
>
> ...the main thread closes the fuse dev fd before FSCONFIG_CMD_CREATE.
> What if that main thread needs to use the fuse dev fd after this point?
> Or, what if userspace doesn't cooperate and unshare()/close()?  Can this
> hang be broken by kill -9, at least?

No, only

  echo > /sys/fs/fuse/connections/NN/abort

>
> >  b) only reference the fuse_dev from fs_context not the device file itself
>
> Can you set an abort timeout on the FUSE_INIT request?

I don't like timeouts, but yes, we could.

>
> Perhaps my broader question is, what /does/ happen if a fuse server
> thread starts processing a request and crashes out before replying?  I
> guess that means the request is never completed, but in the case of
> !(synchronous FUSE_INIT) we'd just see the whole server terminate, which
> would then release the fuse dev fd?

Right.

Thanks,
Miklos

Re: [PATCH v2 4/6] fuse: clean up device cloning

[Miklos Szeredi]

On Sat, 14 Mar 2026 at 00:59, Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Thu, Mar 12, 2026 at 08:40:02PM +0100, Miklos Szeredi wrote:
> >  - fuse_mutex is not needed for device cloning, because fuse_dev_install()
> >    uses cmpxcg() to set fud->fc, which prevents races between clone/mount
> >    or clone/clone.  This makes the logic simpler
> >
> >  - Drop fc->dev_count.  This is only used to check in release if the device
> >    is the last clone, but checking list_empty(&fc->devices) is equivalent
> >    after removing the released device from the list.  Removing the fuse_dev
> >    before calling fuse_abort_conn() is okay, since the processing and io
> >    lists are now empty for this device.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >  fs/fuse/dev.c    | 44 ++++++++++++++++----------------------------
> >  fs/fuse/fuse_i.h |  3 ---
> >  fs/fuse/inode.c  |  1 -
> >  3 files changed, 16 insertions(+), 32 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 3adf6bd38c9b..18cc844cf290 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -2543,14 +2543,15 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> >
> >               fuse_dev_end_requests(&to_end);
> >
> > +             spin_lock(&fc->lock);
> > +             list_del(&fud->entry);
> > +             spin_unlock(&fc->lock);
> > +
> >               /* Are we the last open device? */
> > -             if (atomic_dec_and_test(&fc->dev_count)) {
> > +             if (list_empty(&fc->devices)) {
>
> If you have two threads closing their cloned fuse dev fds, can they both
> see an empty list here and abort the connection?  I think they can, but
> it's benign because the abort takes its own spinlock.

You're right.  Cleaner to move that check inside the spinlocked region.

Thanks,
Miklos

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Darrick J. Wong]

On Mon, Mar 16, 2026 at 12:29:53PM +0100, Miklos Szeredi wrote:
> On Sat, 14 Mar 2026 at 00:09, Darrick J. Wong <djwong@kernel.org> wrote:
> 
> > Hrmm, is this https://github.com/libfuse/libfuse/pull/1367 ?
> 
> Right.
> 
> > Which syscall causes the synchronous FUSE_INIT to be sent?  Is it
> > FSCONFIG_CMD_CREATE?
> 
> Yes.
> 
> > > The reason is that while all other threads will exit, the mounting
> > > thread (or process) will keep the device fd open, which will prevent
> > > an abort from happening.
> >
> > So I guess what's happening here is that the main thread calls
> > FSCONFIG_CMD_CREATE, which sends the synchronous FUSE_INIT to the worker
> > pool.  One of the worker threads starts working on the FUSE_INIT reply
> > and crashes.  All the *workers* terminate and close the fuse dev fd,
> > leaving just the main thread.
> >
> > AFAICT, the main thread is stuck in fsconfig() here:
> >
> >         /* Any signal may interrupt this */
> >                 err = wait_event_interruptible(req->waitq,
> >                                 test_bit(FR_FINISHED, &req->flags))
> >
> > so there's still an open ref to the fuse dev fd, which prevents anyone
> > from aborting the FUSE_INIT request so that FR_FINISH gets set?
> 
> Yes.
> 
> >  And now
> > we're just hosed?  Wouldn't the SIGCHLD interrupt the wait?  Or are we
> > stuck someplace else?
> 
> The FUSE_INIT request is in FR_SENT state, and fuse doesn't allow
> interrupting such requests without the cooperation of the server.
> 
> We could special case FUSE_INIT (and probably a number of other
> request types) that are safe to kill without the server's consent.
> Will look into this.

I wonder if there's a way to have a wait_event_killable that will wake
up if the process exits without being killed by a signal?  Or does it
already do that...

> >
> > > This is a regression from the async mount case, where the mount was done
> > > first, and the FUSE_INIT processing afterwards, in which case there's no
> > > such recursive syscall keeping the fd open.
> >
> > Is this hang possible if you're using mount(2) with synchronous
> > FUSE_INIT?
> 
> Yes.

Yikes.  I guess at least there's the echo > .../abort solution below.

> > > The solution is twofold:
> > >
> > >  a) use unshare(CLONE_FILES) in the mounting thread
> >
> > Is this after starting up the worker threads?  I guess that means the
> > worker threads retain their fuse dev fds even though...
> >
> > >     and close the device fd
> > >     after fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", ...)
> >
> > ...the main thread closes the fuse dev fd before FSCONFIG_CMD_CREATE.
> > What if that main thread needs to use the fuse dev fd after this point?
> > Or, what if userspace doesn't cooperate and unshare()/close()?  Can this
> > hang be broken by kill -9, at least?
> 
> No, only
> 
>   echo > /sys/fs/fuse/connections/NN/abort
> 
> >
> > >  b) only reference the fuse_dev from fs_context not the device file itself
> >
> > Can you set an abort timeout on the FUSE_INIT request?
> 
> I don't like timeouts, but yes, we could.

<nod> Or *some* means of figuring out that one of the other threads has
crashed the process, so we might as well cancel the wait_event?

> >
> > Perhaps my broader question is, what /does/ happen if a fuse server
> > thread starts processing a request and crashes out before replying?  I
> > guess that means the request is never completed, but in the case of
> > !(synchronous FUSE_INIT) we'd just see the whole server terminate, which
> > would then release the fuse dev fd?
> 
> Right.

(I forget, what was the purpose of synchronous FUSE_INIT?)

--D

> Thanks,
> Miklos

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Joanne Koong]

On Tue, Mar 17, 2026 at 4:40 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Mon, Mar 16, 2026 at 12:29:53PM +0100, Miklos Szeredi wrote:
> > On Sat, 14 Mar 2026 at 00:09, Darrick J. Wong <djwong@kernel.org> wrote:
> > >
> > > Perhaps my broader question is, what /does/ happen if a fuse server
> > > thread starts processing a request and crashes out before replying?  I
> > > guess that means the request is never completed, but in the case of
> > > !(synchronous FUSE_INIT) we'd just see the whole server terminate, which
> > > would then release the fuse dev fd?
> >
> > Right.
>
> (I forget, what was the purpose of synchronous FUSE_INIT?)

"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."

from https://lore.kernel.org/linux-fsdevel/20250827110004.584582-1-mszeredi@redhat.com/


Thanks,
Joanne

>
> --D
>
> > Thanks,
> > Miklos
>

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Bernd Schubert]

On 3/18/26 00:49, Joanne Koong wrote:
> On Tue, Mar 17, 2026 at 4:40 PM Darrick J. Wong <djwong@kernel.org> wrote:
>>
>> On Mon, Mar 16, 2026 at 12:29:53PM +0100, Miklos Szeredi wrote:
>>> On Sat, 14 Mar 2026 at 00:09, Darrick J. Wong <djwong@kernel.org> wrote:
>>>>
>>>> Perhaps my broader question is, what /does/ happen if a fuse server
>>>> thread starts processing a request and crashes out before replying?  I
>>>> guess that means the request is never completed, but in the case of
>>>> !(synchronous FUSE_INIT) we'd just see the whole server terminate, which
>>>> would then release the fuse dev fd?
>>>
>>> Right.
>>
>> (I forget, what was the purpose of synchronous FUSE_INIT?)
> 
> "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."
> 
> from https://lore.kernel.org/linux-fsdevel/20250827110004.584582-1-mszeredi@redhat.com/

Hi Joanne,

the libfuse work for sync FUSE_INIT and selinux support is here

https://github.com/libfuse/libfuse/pull/1367

I think Kevin had already tested that selinux works, so far I have only
tested the basic mount. Currently doing more refactoring so that
fusermount link to the new lib/mount_fsmount.c.
Should be done by tomorrow (will send out new reduced queue io-uring
patches once I'm through and then eventually work on letting the
application own the rings instead of libfuse - tight plan till the end
of the month ;) ).


Thanks,
Bernd

Re: [PATCH v2 3/6] fuse: don't require /dev/fuse fd to be kept open during mount

[Joanne Koong]

On Wed, Mar 18, 2026 at 3:15 PM Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
>
> On 3/18/26 00:49, Joanne Koong wrote:
> > On Tue, Mar 17, 2026 at 4:40 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >>
> >> On Mon, Mar 16, 2026 at 12:29:53PM +0100, Miklos Szeredi wrote:
> >>> On Sat, 14 Mar 2026 at 00:09, Darrick J. Wong <djwong@kernel.org> wrote:
> >>>>
> >>>> Perhaps my broader question is, what /does/ happen if a fuse server
> >>>> thread starts processing a request and crashes out before replying?  I
> >>>> guess that means the request is never completed, but in the case of
> >>>> !(synchronous FUSE_INIT) we'd just see the whole server terminate, which
> >>>> would then release the fuse dev fd?
> >>>
> >>> Right.
> >>
> >> (I forget, what was the purpose of synchronous FUSE_INIT?)
> >
> > "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."
> >
> > from https://lore.kernel.org/linux-fsdevel/20250827110004.584582-1-mszeredi@redhat.com/
>
> Hi Joanne,
>
> the libfuse work for sync FUSE_INIT and selinux support is here
>
> https://github.com/libfuse/libfuse/pull/1367
>
> I think Kevin had already tested that selinux works, so far I have only
> tested the basic mount. Currently doing more refactoring so that
> fusermount link to the new lib/mount_fsmount.c.
> Should be done by tomorrow (will send out new reduced queue io-uring
> patches once I'm through and then eventually work on letting the
> application own the rings instead of libfuse - tight plan till the end
> of the month ;) ).

Great, really looking forward to seeing the newest version of the
reduced queue io-uring patches.

Thanks,
Joanne
>
>
> Thanks,
> Bernd

Re: [PATCH v2 1/6] fuse: create fuse_dev on /dev/fuse open instead of mount

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 4525 bytes --]

On Thu, Mar 12, 2026 at 08:39:59PM +0100, Miklos Szeredi wrote:
> Allocate struct fuse_dev when opening the device.  This means that unlike
> before, ->private_data is always set to a valid pointer.
> 
> The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
> replaced with a simple bool sync_init member.

Since Monday we've been seeing problems with the read_all_dev LTP
testcase running -next on a range of arm64 platforms, these bisect to
this commit.  The test reports:

11468 01:06:07.273320  tst_test.c:2021: TINFO: LTP version: 20250930
11469 01:06:07.284741  tst_test.c:2024: TINFO: Tested kernel: 7.0.0-rc4-next-20260316 #1 SMP PREEMPT Mon Mar 16 20:19:24 UTC 2026 aarch64
11470 01:06:07.285022  tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
11471 01:06:07.296067  tst_test.c:1842: TINFO: Overall timeout per run is 0h 02m 10s
11472 01:06:07.296580  read_all.c:601: TINFO: Worker timeout set to 10% of runtime: 1000ms
11473 01:06:07.296818  Test timeouted, sending SIGKILL!
11474 01:06:07.307439  tst_test.c:1905: TINFO: Killed the leftover descendant processes
11475 01:06:07.318985  tst_test.c:1914: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
11476 01:06:07.319263  tst_test.c:1916: TBROK: Test killed! (timeout?)
11477 01:06:07.319486  
11478 01:06:07.319690  Summary:
11479 01:06:07.319887  passed   0
11480 01:06:07.320076  failed   0
11481 01:06:07.320262  broken   1

The test does a read of every file under /dev, the actual binary is
read_all and the command is:

   read_all -d /dev -p -q -r 3

so the connection with a change to a change in behaviour when opening
/dev/fuse seems plausible though I didn't investigate further than that
and checking that the bisect looks smooth.

bisect log:

git bisect start
# status: waiting for both good and bad commits
# bad: [8e42d2514a7e8eb8d740d0ba82339dd6c0b6463f] Add linux-next specific files for 20260318
git bisect bad 8e42d2514a7e8eb8d740d0ba82339dd6c0b6463f
# status: waiting for good commit(s), bad commit known
# good: [2430154a93764c58b8ee9b6f7e5b3563ff452baa] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
git bisect good 2430154a93764c58b8ee9b6f7e5b3563ff452baa
# bad: [1ec9b3fbfc367f64a29f3335ae9879202993b7bc] Merge branch 'nand/next' of https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
git bisect bad 1ec9b3fbfc367f64a29f3335ae9879202993b7bc
# bad: [91392dfbff751ca86443a6f3ce7c46143fecdecd] Merge branch 'fs-next' of linux-next
git bisect bad 91392dfbff751ca86443a6f3ce7c46143fecdecd
# good: [41512607505955aaf57cc6899aa548ae75ab81e6] Merge branch 'for-next' of https://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect good 41512607505955aaf57cc6899aa548ae75ab81e6
# bad: [515b2513dabbdec4536c8c1b8de56271efe800f6] Merge branch 'for-next' of https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git
git bisect bad 515b2513dabbdec4536c8c1b8de56271efe800f6
# bad: [2115fbae3f0b9ad858373ed31ee9af0b25a63d6d] Merge branch 'jfs-next' of https://github.com/kleikamp/linux-shaggy.git
git bisect bad 2115fbae3f0b9ad858373ed31ee9af0b25a63d6d
# good: [41019679bc4f7ed6b072a83ba264908bc88752dd] Merge branch 'for-next-next-v7.0-20260316' into for-next-20260316
git bisect good 41019679bc4f7ed6b072a83ba264908bc88752dd
# good: [a0597d24fd296252e40643e95ca22c752f246158] Merge branch 'for_next' of https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git
git bisect good a0597d24fd296252e40643e95ca22c752f246158
# bad: [6dcceeb728567a61fbad781c13828b638b32bc16] fuse: support FSCONFIG_SET_FD for "fd" option
git bisect bad 6dcceeb728567a61fbad781c13828b638b32bc16
# good: [25307ca50b815c14a21f82fc5b10e8a621af32ad] fuse: simplify logic in fuse_notify_store() and fuse_retrieve()
git bisect good 25307ca50b815c14a21f82fc5b10e8a621af32ad
# bad: [3379620923d3e59b6767605a0f11d13b37d72a0c] fuse: create fuse_dev on /dev/fuse open instead of mount
git bisect bad 3379620923d3e59b6767605a0f11d13b37d72a0c
# good: [8d306cbffc2ee0f3251c81d574aa3451ef21cd5a] fuse: use offset_in_page() for page offset calculations
git bisect good 8d306cbffc2ee0f3251c81d574aa3451ef21cd5a
# good: [5a6baf204610589f8a5b5a1cd69d1fe661d9d3cd] fuse: fix uninit-value in fuse_dentry_revalidate()
git bisect good 5a6baf204610589f8a5b5a1cd69d1fe661d9d3cd
# first bad commit: [3379620923d3e59b6767605a0f11d13b37d72a0c] fuse: create fuse_dev on /dev/fuse open instead of mount

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]