[PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Chen Linxuan via B4 Relay]

Please review this patch series carefully. I am new to kernel
development and I am not quite sure if I have followed the best
practices, especially in terms of seq_file, error handling and locking.
I would appreciate any feedback.

I have do some simply testing using libfuse example [1]. It seems to
work well.

[1]: https://github.com/libfuse/libfuse/blob/master/example/passthrough_hp.cc

Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
Changes in v3:
- Apply some suggestions from Amir Goldstein
- Link to v2: https://lore.kernel.org/r/20250508-fusectl-backing-files-v2-0-62f564e76984@uniontech.com

Changes in v2:
- Void using seq_file private field as it seems that we can just simply
  returning fuse_backing_files_seq_state in start() and next()
- Apply some suggestions from Amir Goldstein:
  - Use idr_get_next() for iteration
  - Do fuse_backing_get/put(fb) around dereferencing fb->file
- Update fdinfo of fuse files
- Link to v1: https://lore.kernel.org/r/20250507032926.377076-2-chenlinxuan@uniontech.com

---
Chen Linxuan (3):
      fs: fuse: add const qualifier to fuse_ctl_file_conn_get()
      fs: fuse: add backing_files control file
      fs: fuse: add more information to fdinfo

 fs/fuse/control.c | 157 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/fuse/file.c    |  20 +++++++
 fs/fuse/fuse_i.h  |   2 +-
 3 files changed, 165 insertions(+), 14 deletions(-)
---
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
change-id: 20250508-fusectl-backing-files-6810d290e798

Best regards,
-- 
Chen Linxuan <chenlinxuan@uniontech.com>

[PATCH v3 1/3] fs: fuse: add const qualifier to fuse_ctl_file_conn_get()

[Chen Linxuan via B4 Relay]

From: Chen Linxuan <chenlinxuan@uniontech.com>

Add const qualifier to the file parameter in fuse_ctl_file_conn_get
function to indicate that this function does not modify the passed file
object. This improves code clarity and type safety by making the API
contract more explicit.

Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
 fs/fuse/control.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index 2a730d88cc3bdb50ea1f8a3185faad5f05fc6e74..f0874403b1f7c91571f38e4ae9f8cebe259f7dd1 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -20,7 +20,7 @@
  */
 static struct super_block *fuse_control_sb;
 
-static struct fuse_conn *fuse_ctl_file_conn_get(struct file *file)
+static struct fuse_conn *fuse_ctl_file_conn_get(const struct file *file)
 {
 	struct fuse_conn *fc;
 	mutex_lock(&fuse_mutex);

-- 
2.43.0

Re: [PATCH v3 1/3] fs: fuse: add const qualifier to fuse_ctl_file_conn_get()

[Amir Goldstein]

On Fri, May 9, 2025 at 8:34 AM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@uniontech.com>
>
> Add const qualifier to the file parameter in fuse_ctl_file_conn_get
> function to indicate that this function does not modify the passed file
> object. This improves code clarity and type safety by making the API
> contract more explicit.
>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>

Feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/fuse/control.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> index 2a730d88cc3bdb50ea1f8a3185faad5f05fc6e74..f0874403b1f7c91571f38e4ae9f8cebe259f7dd1 100644
> --- a/fs/fuse/control.c
> +++ b/fs/fuse/control.c
> @@ -20,7 +20,7 @@
>   */
>  static struct super_block *fuse_control_sb;
>
> -static struct fuse_conn *fuse_ctl_file_conn_get(struct file *file)
> +static struct fuse_conn *fuse_ctl_file_conn_get(const struct file *file)
>  {
>         struct fuse_conn *fc;
>         mutex_lock(&fuse_mutex);
>
> --
> 2.43.0
>
>

[PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan via B4 Relay]

From: Chen Linxuan <chenlinxuan@uniontech.com>

Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
that exposes the paths of all backing files currently being used in
FUSE mount points. This is particularly valuable for tracking and
debugging files used in FUSE passthrough mode.

This approach is similar to how fixed files in io_uring expose their
status through fdinfo, providing administrators with visibility into
backing file usage. By making backing files visible through the FUSE
control filesystem, administrators can monitor which files are being
used for passthrough operations and can force-close them if needed by
aborting the connection.

This exposure of backing files information is an important step towards
potentially relaxing CAP_SYS_ADMIN requirements for certain passthrough
operations in the future, allowing for better security analysis of
passthrough usage patterns.

The control file is implemented using the seq_file interface for
efficient handling of potentially large numbers of backing files.
Access permissions are set to read-only (0400) as this is an
informational interface.

FUSE_CTL_NUM_DENTRIES has been increased from 5 to 6 to accommodate the
additional control file.

Some related discussions can be found at links below.

Link: https://lore.kernel.org/all/4b64a41c-6167-4c02-8bae-3021270ca519@fastmail.fm/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e
Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@mail.gmail.com/
Cc: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
 fs/fuse/control.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++-----
 fs/fuse/fuse_i.h  |   2 +-
 2 files changed, 144 insertions(+), 13 deletions(-)

diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..6333fffec85bd562dc9e86ba7cbf88b8bc2d68ce 100644
--- a/fs/fuse/control.c
+++ b/fs/fuse/control.c
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/fs_context.h>
+#include <linux/seq_file.h>
 
 #define FUSE_CTL_SUPER_MAGIC 0x65735543
 
@@ -180,6 +181,135 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
 	return ret;
 }
 
+struct fuse_backing_files_seq_state {
+	struct fuse_conn *fc;
+	int backing_id;
+};
+
+static void fuse_backing_files_seq_state_free(struct fuse_backing_files_seq_state *state)
+{
+	fuse_conn_put(state->fc);
+	kvfree(state);
+}
+
+static void *fuse_backing_files_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	struct fuse_backing *fb;
+	struct fuse_backing_files_seq_state *state;
+	struct fuse_conn *fc;
+	int backing_id;
+	void *ret;
+
+	fc = fuse_ctl_file_conn_get(seq->file);
+	if (!fc)
+		return ERR_PTR(-ENOTCONN);
+
+	backing_id = *pos;
+
+	rcu_read_lock();
+
+	fb = idr_get_next(&fc->backing_files_map, &backing_id);
+
+	rcu_read_unlock();
+
+	if (!fb) {
+		ret = NULL;
+		goto err;
+	}
+
+	state = kmalloc(sizeof(*state), GFP_KERNEL);
+	if (!state) {
+		ret = ERR_PTR(-ENOMEM);
+		goto err;
+	}
+
+	state->fc = fc;
+	state->backing_id = backing_id;
+	*pos = backing_id;
+
+	ret = state;
+	return ret;
+
+err:
+	fuse_conn_put(fc);
+	return ret;
+}
+
+static void *fuse_backing_files_seq_next(struct seq_file *seq, void *v,
+					 loff_t *pos)
+{
+	struct fuse_backing_files_seq_state *state = v;
+	struct fuse_backing *fb;
+
+	state->backing_id++;
+
+	rcu_read_lock();
+
+	fb = idr_get_next(&state->fc->backing_files_map, &state->backing_id);
+
+	rcu_read_unlock();
+
+	if (!fb) {
+		fuse_backing_files_seq_state_free(state);
+		return NULL;
+	}
+
+	*pos = state->backing_id;
+
+	return state;
+}
+
+static int fuse_backing_files_seq_show(struct seq_file *seq, void *v)
+{
+	struct fuse_backing_files_seq_state *state = v;
+	struct fuse_conn *fc = state->fc;
+	struct fuse_backing *fb;
+
+	rcu_read_lock();
+
+	fb = idr_find(&fc->backing_files_map, state->backing_id);
+	fb = fuse_backing_get(fb);
+
+	rcu_read_unlock();
+
+	if (!fb)
+		return 0;
+
+	if (fb->file) {
+		seq_printf(seq, "%5u: ", state->backing_id);
+		seq_file_path(seq, fb->file, " \t\n\\");
+		seq_puts(seq, "\n");
+	}
+
+	fuse_backing_put(fb);
+	return 0;
+}
+
+static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v)
+{
+	if (v)
+		fuse_backing_files_seq_state_free(v);
+}
+
+static const struct seq_operations fuse_backing_files_seq_ops = {
+	.start = fuse_backing_files_seq_start,
+	.next = fuse_backing_files_seq_next,
+	.stop = fuse_backing_files_seq_stop,
+	.show = fuse_backing_files_seq_show,
+};
+
+static int fuse_backing_files_seq_open(struct inode *inode, struct file *file)
+{
+	return seq_open(file, &fuse_backing_files_seq_ops);
+}
+
+static const struct file_operations fuse_conn_backing_files_ops = {
+	.open = fuse_backing_files_seq_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release,
+};
+
 static const struct file_operations fuse_ctl_abort_ops = {
 	.open = nonseekable_open,
 	.write = fuse_conn_abort_write,
@@ -204,8 +334,7 @@ static const struct file_operations fuse_conn_congestion_threshold_ops = {
 
 static struct dentry *fuse_ctl_add_dentry(struct dentry *parent,
 					  struct fuse_conn *fc,
-					  const char *name,
-					  int mode, int nlink,
+					  const char *name, int mode, int nlink,
 					  const struct inode_operations *iop,
 					  const struct file_operations *fop)
 {
@@ -262,20 +391,22 @@ int fuse_ctl_add_conn(struct fuse_conn *fc)
 	if (!parent)
 		goto err;
 
-	if (!fuse_ctl_add_dentry(parent, fc, "waiting", S_IFREG | 0400, 1,
-				 NULL, &fuse_ctl_waiting_ops) ||
-	    !fuse_ctl_add_dentry(parent, fc, "abort", S_IFREG | 0200, 1,
-				 NULL, &fuse_ctl_abort_ops) ||
+	if (!fuse_ctl_add_dentry(parent, fc, "waiting", S_IFREG | 0400, 1, NULL,
+				 &fuse_ctl_waiting_ops) ||
+	    !fuse_ctl_add_dentry(parent, fc, "abort", S_IFREG | 0200, 1, NULL,
+				 &fuse_ctl_abort_ops) ||
 	    !fuse_ctl_add_dentry(parent, fc, "max_background", S_IFREG | 0600,
 				 1, NULL, &fuse_conn_max_background_ops) ||
 	    !fuse_ctl_add_dentry(parent, fc, "congestion_threshold",
 				 S_IFREG | 0600, 1, NULL,
-				 &fuse_conn_congestion_threshold_ops))
+				 &fuse_conn_congestion_threshold_ops) ||
+	    !fuse_ctl_add_dentry(parent, fc, "backing_files", S_IFREG | 0400, 1,
+				 NULL, &fuse_conn_backing_files_ops))
 		goto err;
 
 	return 0;
 
- err:
+err:
 	fuse_ctl_remove_conn(fc);
 	return -ENOMEM;
 }
@@ -335,7 +466,7 @@ static int fuse_ctl_get_tree(struct fs_context *fsc)
 }
 
 static const struct fs_context_operations fuse_ctl_context_ops = {
-	.get_tree	= fuse_ctl_get_tree,
+	.get_tree = fuse_ctl_get_tree,
 };
 
 static int fuse_ctl_init_fs_context(struct fs_context *fsc)
@@ -358,10 +489,10 @@ static void fuse_ctl_kill_sb(struct super_block *sb)
 }
 
 static struct file_system_type fuse_ctl_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "fusectl",
+	.owner = THIS_MODULE,
+	.name = "fusectl",
 	.init_fs_context = fuse_ctl_init_fs_context,
-	.kill_sb	= fuse_ctl_kill_sb,
+	.kill_sb = fuse_ctl_kill_sb,
 };
 MODULE_ALIAS_FS("fusectl");
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d56d4fd956db99ecd93052a9655428664882cb72..2830b05bb0928e9b30f9905d70fc3ec1ef11c2a5 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -46,7 +46,7 @@
 #define FUSE_NAME_MAX (PATH_MAX - 1)
 
 /** Number of dentries for each connection in the control filesystem */
-#define FUSE_CTL_NUM_DENTRIES 5
+#define FUSE_CTL_NUM_DENTRIES 6
 
 /* Frequency (in seconds) of request timeout checks, if opted into */
 #define FUSE_TIMEOUT_TIMER_FREQ 15

-- 
2.43.0

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Amir Goldstein]

On Fri, May 9, 2025 at 8:34 AM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@uniontech.com>
>
> Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> that exposes the paths of all backing files currently being used in
> FUSE mount points. This is particularly valuable for tracking and
> debugging files used in FUSE passthrough mode.
>
> This approach is similar to how fixed files in io_uring expose their
> status through fdinfo, providing administrators with visibility into
> backing file usage. By making backing files visible through the FUSE
> control filesystem, administrators can monitor which files are being
> used for passthrough operations and can force-close them if needed by
> aborting the connection.
>
> This exposure of backing files information is an important step towards
> potentially relaxing CAP_SYS_ADMIN requirements for certain passthrough
> operations in the future, allowing for better security analysis of
> passthrough usage patterns.
>
> The control file is implemented using the seq_file interface for
> efficient handling of potentially large numbers of backing files.
> Access permissions are set to read-only (0400) as this is an
> informational interface.
>
> FUSE_CTL_NUM_DENTRIES has been increased from 5 to 6 to accommodate the
> additional control file.
>
> Some related discussions can be found at links below.
>
> Link: https://lore.kernel.org/all/4b64a41c-6167-4c02-8bae-3021270ca519@fastmail.fm/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@mail.gmail.com/
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---

Looks good!
With minor nits fixed, please feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
(instead of Cc:)

...

>  static const struct fs_context_operations fuse_ctl_context_ops = {
> -       .get_tree       = fuse_ctl_get_tree,
> +       .get_tree = fuse_ctl_get_tree,
>  };
>
>  static int fuse_ctl_init_fs_context(struct fs_context *fsc)
> @@ -358,10 +489,10 @@ static void fuse_ctl_kill_sb(struct super_block *sb)
>  }
>
>  static struct file_system_type fuse_ctl_fs_type = {
> -       .owner          = THIS_MODULE,
> -       .name           = "fusectl",
> +       .owner = THIS_MODULE,
> +       .name = "fusectl",
>         .init_fs_context = fuse_ctl_init_fs_context,
> -       .kill_sb        = fuse_ctl_kill_sb,
> +       .kill_sb = fuse_ctl_kill_sb,
>  };

Please undo these whitespace changes.

Thanks,
Amir.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Fri, May 9, 2025 at 3:20 PM Amir Goldstein <amir73il@gmail.com> wrote:
> Please undo these whitespace changes.

Sorry for that, I run clang-format somehow. They will be removed in V4.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Fri, May 9, 2025 at 2:34 PM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@uniontech.com>
>
> Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> that exposes the paths of all backing files currently being used in
> FUSE mount points. This is particularly valuable for tracking and
> debugging files used in FUSE passthrough mode.
>
> This approach is similar to how fixed files in io_uring expose their
> status through fdinfo, providing administrators with visibility into
> backing file usage. By making backing files visible through the FUSE
> control filesystem, administrators can monitor which files are being
> used for passthrough operations and can force-close them if needed by
> aborting the connection.
>
> This exposure of backing files information is an important step towards
> potentially relaxing CAP_SYS_ADMIN requirements for certain passthrough
> operations in the future, allowing for better security analysis of
> passthrough usage patterns.
>
> The control file is implemented using the seq_file interface for
> efficient handling of potentially large numbers of backing files.
> Access permissions are set to read-only (0400) as this is an
> informational interface.
>
> FUSE_CTL_NUM_DENTRIES has been increased from 5 to 6 to accommodate the
> additional control file.
>
> Some related discussions can be found at links below.
>
> Link: https://lore.kernel.org/all/4b64a41c-6167-4c02-8bae-3021270ca519@fastmail.fm/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@mail.gmail.com/
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---
>  fs/fuse/control.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++-----
>  fs/fuse/fuse_i.h  |   2 +-
>  2 files changed, 144 insertions(+), 13 deletions(-)
>
> diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..6333fffec85bd562dc9e86ba7cbf88b8bc2d68ce 100644
> --- a/fs/fuse/control.c
> +++ b/fs/fuse/control.c
> @@ -11,6 +11,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/fs_context.h>
> +#include <linux/seq_file.h>
>
>  #define FUSE_CTL_SUPER_MAGIC 0x65735543
>
> @@ -180,6 +181,135 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
>         return ret;
>  }
>
> +struct fuse_backing_files_seq_state {
> +       struct fuse_conn *fc;
> +       int backing_id;
> +};

As mentioned in the previous v2 related discussion
backing_id is maintained in state because
show() of seq_file doesn't accept the pos parameter.
But actually we can get the pos in the show() function
via the index field of struct seq_file.
The softnet_seq_show() function in net-procfs.c are doing this.
I'm not really sure if it would be better to implement it this way.

> +
> +static void fuse_backing_files_seq_state_free(struct fuse_backing_files_seq_state *state)
> +{
> +       fuse_conn_put(state->fc);
> +       kvfree(state);
> +}
> +
> +static void *fuse_backing_files_seq_start(struct seq_file *seq, loff_t *pos)
> +{
> +       struct fuse_backing *fb;
> +       struct fuse_backing_files_seq_state *state;
> +       struct fuse_conn *fc;
> +       int backing_id;
> +       void *ret;
> +
> +       fc = fuse_ctl_file_conn_get(seq->file);

I'm not sure if I should get fc in fuse_backing_files_seq_start here
and handle fc as (part of) the seq_file iterator.
Or should I get the fc in fuse_backing_files_seq_open
and store the fc in the private field of the seq_file more appropriately.
I guess the difference isn't that big?

> +       if (!fc)
> +               return ERR_PTR(-ENOTCONN);
> +
> +       backing_id = *pos;
> +
> +       rcu_read_lock();
> +
> +       fb = idr_get_next(&fc->backing_files_map, &backing_id);
> +
> +       rcu_read_unlock();
> +
> +       if (!fb) {
> +               ret = NULL;
> +               goto err;
> +       }
> +
> +       state = kmalloc(sizeof(*state), GFP_KERNEL);
> +       if (!state) {
> +               ret = ERR_PTR(-ENOMEM);
> +               goto err;
> +       }
> +
> +       state->fc = fc;
> +       state->backing_id = backing_id;
> +       *pos = backing_id;
> +
> +       ret = state;
> +       return ret;
> +
> +err:
> +       fuse_conn_put(fc);
> +       return ret;
> +}
> +
> +static void *fuse_backing_files_seq_next(struct seq_file *seq, void *v,
> +                                        loff_t *pos)
> +{
> +       struct fuse_backing_files_seq_state *state = v;
> +       struct fuse_backing *fb;
> +
> +       state->backing_id++;
> +
> +       rcu_read_lock();
> +
> +       fb = idr_get_next(&state->fc->backing_files_map, &state->backing_id);
> +
> +       rcu_read_unlock();
> +
> +       if (!fb) {
> +               fuse_backing_files_seq_state_free(state);
> +               return NULL;
> +       }
> +
> +       *pos = state->backing_id;
> +
> +       return state;
> +}
> +
> +static int fuse_backing_files_seq_show(struct seq_file *seq, void *v)
> +{
> +       struct fuse_backing_files_seq_state *state = v;
> +       struct fuse_conn *fc = state->fc;
> +       struct fuse_backing *fb;
> +
> +       rcu_read_lock();
> +
> +       fb = idr_find(&fc->backing_files_map, state->backing_id);
> +       fb = fuse_backing_get(fb);
> +
> +       rcu_read_unlock();
> +
> +       if (!fb)
> +               return 0;
> +
> +       if (fb->file) {
> +               seq_printf(seq, "%5u: ", state->backing_id);
> +               seq_file_path(seq, fb->file, " \t\n\\");
> +               seq_puts(seq, "\n");
> +       }
> +
> +       fuse_backing_put(fb);
> +       return 0;
> +}
> +
> +static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v)
> +{
> +       if (v)
> +               fuse_backing_files_seq_state_free(v);
> +}
> +
> +static const struct seq_operations fuse_backing_files_seq_ops = {
> +       .start = fuse_backing_files_seq_start,
> +       .next = fuse_backing_files_seq_next,
> +       .stop = fuse_backing_files_seq_stop,
> +       .show = fuse_backing_files_seq_show,
> +};
> +
> +static int fuse_backing_files_seq_open(struct inode *inode, struct file *file)
> +{
> +       return seq_open(file, &fuse_backing_files_seq_ops);
> +}
> +
> +static const struct file_operations fuse_conn_backing_files_ops = {
> +       .open = fuse_backing_files_seq_open,
> +       .read = seq_read,
> +       .llseek = seq_lseek,
> +       .release = seq_release,
> +};
> +

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Amir Goldstein]

On Fri, May 9, 2025 at 9:39 AM Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> On Fri, May 9, 2025 at 2:34 PM Chen Linxuan via B4 Relay
> <devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
> >
> > From: Chen Linxuan <chenlinxuan@uniontech.com>
> >
> > Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> > that exposes the paths of all backing files currently being used in
> > FUSE mount points. This is particularly valuable for tracking and
> > debugging files used in FUSE passthrough mode.
> >
> > This approach is similar to how fixed files in io_uring expose their
> > status through fdinfo, providing administrators with visibility into
> > backing file usage. By making backing files visible through the FUSE
> > control filesystem, administrators can monitor which files are being
> > used for passthrough operations and can force-close them if needed by
> > aborting the connection.
> >
> > This exposure of backing files information is an important step towards
> > potentially relaxing CAP_SYS_ADMIN requirements for certain passthrough
> > operations in the future, allowing for better security analysis of
> > passthrough usage patterns.
> >
> > The control file is implemented using the seq_file interface for
> > efficient handling of potentially large numbers of backing files.
> > Access permissions are set to read-only (0400) as this is an
> > informational interface.
> >
> > FUSE_CTL_NUM_DENTRIES has been increased from 5 to 6 to accommodate the
> > additional control file.
> >
> > Some related discussions can be found at links below.
> >
> > Link: https://lore.kernel.org/all/4b64a41c-6167-4c02-8bae-3021270ca519@fastmail.fm/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e
> > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@mail.gmail.com/
> > Cc: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> > ---
> >  fs/fuse/control.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> >  fs/fuse/fuse_i.h  |   2 +-
> >  2 files changed, 144 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> > index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..6333fffec85bd562dc9e86ba7cbf88b8bc2d68ce 100644
> > --- a/fs/fuse/control.c
> > +++ b/fs/fuse/control.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> >  #include <linux/fs_context.h>
> > +#include <linux/seq_file.h>
> >
> >  #define FUSE_CTL_SUPER_MAGIC 0x65735543
> >
> > @@ -180,6 +181,135 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file,
> >         return ret;
> >  }
> >
> > +struct fuse_backing_files_seq_state {
> > +       struct fuse_conn *fc;
> > +       int backing_id;
> > +};
>
> As mentioned in the previous v2 related discussion
> backing_id is maintained in state because
> show() of seq_file doesn't accept the pos parameter.
> But actually we can get the pos in the show() function
> via the index field of struct seq_file.
> The softnet_seq_show() function in net-procfs.c are doing this.
> I'm not really sure if it would be better to implement it this way.
>

For better clarity of the code and maintainability,
I think that storing backing_id in the state as you did is better.

> > +
> > +static void fuse_backing_files_seq_state_free(struct fuse_backing_files_seq_state *state)
> > +{
> > +       fuse_conn_put(state->fc);
> > +       kvfree(state);
> > +}
> > +
> > +static void *fuse_backing_files_seq_start(struct seq_file *seq, loff_t *pos)
> > +{
> > +       struct fuse_backing *fb;
> > +       struct fuse_backing_files_seq_state *state;
> > +       struct fuse_conn *fc;
> > +       int backing_id;
> > +       void *ret;
> > +
> > +       fc = fuse_ctl_file_conn_get(seq->file);
>
> I'm not sure if I should get fc in fuse_backing_files_seq_start here
> and handle fc as (part of) the seq_file iterator.
> Or should I get the fc in fuse_backing_files_seq_open
> and store the fc in the private field of the seq_file more appropriately.
> I guess the difference isn't that big?

The simpler the better.
I think what you did is fine. I see no reason to change it.

Thanks,
Amir.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Fri, 9 May 2025 at 08:34, Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@uniontech.com>
>
> Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> that exposes the paths of all backing files currently being used in
> FUSE mount points. This is particularly valuable for tracking and
> debugging files used in FUSE passthrough mode.

This is good work, thanks.

My concern is that this is a very fuse specific interface, even though
the problem is more generic: list hidden open files belonging to a
kernel object, but not installed in any fd:

 - SCM_RIGHTS
 - io_uring
 - fuse

So we could have a new syscall or set of syscalls for this purpose.
But that again goes against my "this is not generic enough" pet peeve.

So we had this idea of reusing getxattr and listxattr (or implementing
a new set of syscalls with the same signature) to allow retrieving a
hierarchical set of attributes belonging to a kernel object.  This one
would also fit that pattern, so...

Thoughts?

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Fri, May 9, 2025 at 10:00 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, 9 May 2025 at 08:34, Chen Linxuan via B4 Relay
> <devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
> >
> > From: Chen Linxuan <chenlinxuan@uniontech.com>
> >
> > Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> > that exposes the paths of all backing files currently being used in
> > FUSE mount points. This is particularly valuable for tracking and
> > debugging files used in FUSE passthrough mode.
>
> This is good work, thanks.
>
> My concern is that this is a very fuse specific interface, even though
> the problem is more generic: list hidden open files belonging to a
> kernel object, but not installed in any fd:
>
>  - SCM_RIGHTS
>  - io_uring

Note that io_uring has already exposed information about fixed files
in its fdinfo.

>  - fuse
>
> So we could have a new syscall or set of syscalls for this purpose.
> But that again goes against my "this is not generic enough" pet peeve.
>
> So we had this idea of reusing getxattr and listxattr (or implementing
> a new set of syscalls with the same signature) to allow retrieving a
> hierarchical set of attributes belonging to a kernel object.  This one
> would also fit that pattern, so...
>
> Thoughts?

Using getxattr does make it more generalized, and I think reusing it
is a good choice.
However, I have some questions:
If we use getxattr,

For io_uring, the path could be the corresponding /proc/PID/fd/* of io_uring.
For FUSE, the path could be /sys/fs/fuse/connections/*.
But for SCM_RIGHTS, what would the corresponding path be? Could it be
the fd under procfs of unix domain socket?

I am also uncertain whether there might be similar situations in the future.
Would we really be able to find a suitable path for all of them?

Thanks,
Chen Linxuan

>
> Thanks,
> Miklos
>
>

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Fri, 9 May 2025 at 16:43, Chen Linxuan <chenlinxuan@uniontech.com> wrote:

> Note that io_uring has already exposed information about fixed files
> in its fdinfo.

And that has limitations, since fdinfo lacks a hierarchy.  E.g.
recursively retrieving "hidden files" info is impractical.

> For io_uring, the path could be the corresponding /proc/PID/fd/* of io_uring.
> For FUSE, the path could be /sys/fs/fuse/connections/*.

Since lsof is ultimately interested in the mapping between open files
and processes, I think using  /proc/PID/fd/* for fuse is also useful.

> But for SCM_RIGHTS, what would the corresponding path be? Could it be
> the fd under procfs of unix domain socket?

Right.  But I'm not asking you to implement this, just thinking about
an interface that is generic enough to cover past and possible future
cases.

> I am also uncertain whether there might be similar situations in the future.
> Would we really be able to find a suitable path for all of them?

Open file descriptors are generic enough to represent most kernel
objects that need to be represented in userspace, so I think this is
not a worry.

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Fri, May 9, 2025 at 10:59 PM Miklos Szeredi <miklos@szeredi.hu> wrote:

> And that has limitations, since fdinfo lacks a hierarchy.  E.g.
> recursively retrieving "hidden files" info is impractical.

What does "recursively" refer to in this context?
I am not entirely sure what kind of situation it implies.
Do you mean, for example, when a fixed file is a FUSE fd and it has a
backing file?

Thanks,
Chen Linxuan

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Fri, 9 May 2025 at 17:11, Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> On Fri, May 9, 2025 at 10:59 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> > And that has limitations, since fdinfo lacks a hierarchy.  E.g.
> > recursively retrieving "hidden files" info is impractical.
>
> What does "recursively" refer to in this context?
> I am not entirely sure what kind of situation it implies.
> Do you mean, for example, when a fixed file is a FUSE fd and it has a
> backing file?

Yeah.  I don't see any practical relevance today, but allowing to
retrieve any attribute of these hidden files (fdinfo and hidden files
included) is a good thing that the flat nature of the current fdinfo
inteface makes difficult.

I'd argue that many current uses of fdinfo could be enabled with the
listxattr/getxattr inteface and would improve usablility (no need to
parse the fdinfo file to skip unneeded info).

Thanks,
Miklos
>
> Thanks,
> Chen Linxuan

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Fri, May 9, 2025 at 10:59 PM Miklos Szeredi <miklos@szeredi.hu> wrote:

> Right.  But I'm not asking you to implement this, just thinking about
> an interface that is generic enough to cover past and possible future
> cases.

I am not very familiar with the existing features in the kernel and
their development directions.
For example, I do know about the SCM_RIGHTS functionality,
but if you hadn't mentioned it here,
I wouldn't have realized that they are essentially addressing the same
kind of problem.

I don't think I currently have enough expertise to design an interface
that could even account for future cases, but I will give it a try.

I noticed that the current extended attribute names already use the
namespace.value format.
Perhaps we could reuse this naming scheme and extend it to support
features like nested namespaces.

For instance, in a situation like this:

A fixed file 0 in an io_uring is a FUSE fd.
This FUSE fd belongs to FUSE connection 64.
This FUSE fd has a backing file.
This backing file is actually provided by mnt_id=36.

Running getfattr -m '-' /proc/path/to/the/io_uring/fd could return
something like:

io_uring.fixed_files.0.fuse.conn="64"
io_uring.fixed_files.0.fuse.backing_file.mnt_id="36"
io_uring.fixed_files.0.fuse.backing_file.path="/path/to/real/file"

Here, the mnt_id is included because I believe
resolving /path/to/real/file in user space might be a relatively complex task.
Providing this attribute could make it easier for tools like lsof to work with.

Additionally, the reason I am working on this is that
I want to make FUSE's passthrough functionality work for non-privileged users.

Someone on the mailing list asked why passthrough requires privileges before:
https://lore.kernel.org/all/CAOYeF9V_FM+0iZcsvi22XvHJuXLXP6wUYPwRYfwVFThajww9YA@mail.gmail.com/#t
In response, I submitted a patch that includes documentation explaining this:
https://lore.kernel.org/all/20250507-fuse-passthrough-doc-v2-0-ae7c0dd8bba6@uniontech.com/

Perhaps we could start by discussing that patch.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Sun, 11 May 2025 at 11:56, Chen Linxuan <chenlinxuan@uniontech.com> wrote:

> I noticed that the current extended attribute names already use the
> namespace.value format.
> Perhaps we could reuse this naming scheme and extend it to support
> features like nested namespaces.

Right.  Here's a link to an old and long thread about this:

   https://lore.kernel.org/all/YnEeuw6fd1A8usjj@miu.piliscsaba.redhat.com/#r

>
> For instance, in a situation like this:
>
> A fixed file 0 in an io_uring is a FUSE fd.
> This FUSE fd belongs to FUSE connection 64.
> This FUSE fd has a backing file.
> This backing file is actually provided by mnt_id=36.
>
> Running getfattr -m '-' /proc/path/to/the/io_uring/fd could return
> something like:
>
> io_uring.fixed_files.0.fuse.conn="64"
> io_uring.fixed_files.0.fuse.backing_file.mnt_id="36"
> io_uring.fixed_files.0.fuse.backing_file.path="/path/to/real/file"

Yeah, except listxattr wouldn't be able to properly work in such
cases: it lacks support for hierarchy.

The proposed solution was something like making getxattr on the
"directory" return a listing of child object names.

I.e. getxattr("/proc/123/fd/12", "io_uring.fixed_files.") would return
the list of instantiated fixed file slots, etc...

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Christian Brauner]

On Mon, May 12, 2025 at 10:27:19AM +0200, Miklos Szeredi wrote:
> On Sun, 11 May 2025 at 11:56, Chen Linxuan <chenlinxuan@uniontech.com> wrote:
> 
> > I noticed that the current extended attribute names already use the
> > namespace.value format.
> > Perhaps we could reuse this naming scheme and extend it to support
> > features like nested namespaces.
> 
> Right.  Here's a link to an old and long thread about this:
> 
>    https://lore.kernel.org/all/YnEeuw6fd1A8usjj@miu.piliscsaba.redhat.com/#r
> 
> >
> > For instance, in a situation like this:
> >
> > A fixed file 0 in an io_uring is a FUSE fd.
> > This FUSE fd belongs to FUSE connection 64.
> > This FUSE fd has a backing file.
> > This backing file is actually provided by mnt_id=36.
> >
> > Running getfattr -m '-' /proc/path/to/the/io_uring/fd could return
> > something like:
> >
> > io_uring.fixed_files.0.fuse.conn="64"
> > io_uring.fixed_files.0.fuse.backing_file.mnt_id="36"
> > io_uring.fixed_files.0.fuse.backing_file.path="/path/to/real/file"
> 
> Yeah, except listxattr wouldn't be able to properly work in such
> cases: it lacks support for hierarchy.
> 
> The proposed solution was something like making getxattr on the
> "directory" return a listing of child object names.
> 
> I.e. getxattr("/proc/123/fd/12", "io_uring.fixed_files.") would return
> the list of instantiated fixed file slots, etc...

Sorry, I'm still very much opposed to using the xattr interface for
this. It is as ugly as it can get and it is outright broken in various
ways. And I don't want it used for more stuff in the future especially
for VFS related information such as this.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Amir Goldstein]

On Mon, May 12, 2025 at 10:27 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Sun, 11 May 2025 at 11:56, Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> > I noticed that the current extended attribute names already use the
> > namespace.value format.
> > Perhaps we could reuse this naming scheme and extend it to support
> > features like nested namespaces.
>
> Right.  Here's a link to an old and long thread about this:
>
>    https://lore.kernel.org/all/YnEeuw6fd1A8usjj@miu.piliscsaba.redhat.com/#r
>

The way I see it is, generic vs. specialized have pros and cons
There is no clear winner.
Therefore, investing time on the getxattr() direction without consensus
with vfs maintainer is not wise IMO.

> >
> > For instance, in a situation like this:
> >
> > A fixed file 0 in an io_uring is a FUSE fd.
> > This FUSE fd belongs to FUSE connection 64.
> > This FUSE fd has a backing file.
> > This backing file is actually provided by mnt_id=36.
> >
> > Running getfattr -m '-' /proc/path/to/the/io_uring/fd could return
> > something like:
> >
> > io_uring.fixed_files.0.fuse.conn="64"
> > io_uring.fixed_files.0.fuse.backing_file.mnt_id="36"
> > io_uring.fixed_files.0.fuse.backing_file.path="/path/to/real/file"
>
> Yeah, except listxattr wouldn't be able to properly work in such
> cases: it lacks support for hierarchy.
>
> The proposed solution was something like making getxattr on the
> "directory" return a listing of child object names.
>
> I.e. getxattr("/proc/123/fd/12", "io_uring.fixed_files.") would return
> the list of instantiated fixed file slots, etc...

The problem I see with this scheme is that it is not generic enough.
If lsof is to support displaying fuse backing files, then it needs to
know specifically about those magic xattrs.

Because lsof only displays information about open files, I think
it would be better to come up with a standard tag in fdinfo for lsof
to recognize, for example:

hidden_file: /path/to/hidden/file
hidden_files_list: /path/to/connections/N/backing_files

and then conform to the same fdinfo standard from fuse_dev_fd,
io_uring_fd, unix_socket_fd.

Making an interface more hierarchic than hidden_files_list:
is useless because lsof traverses all fds anyway to filter by
name pattern and I am very sceptical of anyone trying to
push for an API get_open_fds_by_name_pattern()...

Thanks,
Amir.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Mon, 12 May 2025 at 11:23, Amir Goldstein <amir73il@gmail.com> wrote:

> The way I see it is, generic vs. specialized have pros and cons
> There is no clear winner.
> Therefore, investing time on the getxattr() direction without consensus
> with vfs maintainer is not wise IMO.

AFAIU Christian is hung up about getting a properly sized buffer for the result.

But if the data is inherently variable sized, adding specialized
interface is not going to magically solve that.

Instead we can concentrate on solving the buffer sizing problem
generally, so that all may benefit.

> The problem I see with this scheme is that it is not generic enough.
> If lsof is to support displaying fuse backing files, then it needs to
> know specifically about those magic xattrs.

Yeah, I didn't think that through.  Need some *standard* names.

> Because lsof only displays information about open files, I think
> it would be better to come up with a standard tag in fdinfo for lsof
> to recognize, for example:
>
> hidden_file: /path/to/hidden/file
> hidden_files_list: /path/to/connections/N/backing_files

Ugh.

> Making an interface more hierarchic than hidden_files_list:
> is useless because lsof traverses all fds anyway to filter by
> name pattern and I am very sceptical of anyone trying to
> push for an API get_open_fds_by_name_pattern()...

The problem is that hidden files are hidden, lsof can't traverse them
normally.  It would be good to unhide them in some ways, and for me
that would at least mean that you can

 1) query the path (proc/PID/fd/N link)
 2) query fdinfo
 3) query hidden files

And by recursivity I mean that third point.

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Christian Brauner]

On Mon, May 12, 2025 at 12:06:19PM +0200, Miklos Szeredi wrote:
> On Mon, 12 May 2025 at 11:23, Amir Goldstein <amir73il@gmail.com> wrote:
> 
> > The way I see it is, generic vs. specialized have pros and cons
> > There is no clear winner.
> > Therefore, investing time on the getxattr() direction without consensus
> > with vfs maintainer is not wise IMO.
> 
> AFAIU Christian is hung up about getting a properly sized buffer for the result.

No, the xattr interface is ugly as hell and I don't want it used as a
generic information transportation information interface. And I don't
want a single thing that sets a precedent in that direction.

> But if the data is inherently variable sized, adding specialized
> interface is not going to magically solve that.
> 
> Instead we can concentrate on solving the buffer sizing problem
> generally, so that all may benefit.

The xattr system call as far as I'm concerned is not going to be pimped
to support stuff like that.

> 
> > The problem I see with this scheme is that it is not generic enough.
> > If lsof is to support displaying fuse backing files, then it needs to
> > know specifically about those magic xattrs.
> 
> Yeah, I didn't think that through.  Need some *standard* names.
> 
> > Because lsof only displays information about open files, I think
> > it would be better to come up with a standard tag in fdinfo for lsof
> > to recognize, for example:
> >
> > hidden_file: /path/to/hidden/file
> > hidden_files_list: /path/to/connections/N/backing_files
> 
> Ugh.
> 
> > Making an interface more hierarchic than hidden_files_list:
> > is useless because lsof traverses all fds anyway to filter by
> > name pattern and I am very sceptical of anyone trying to
> > push for an API get_open_fds_by_name_pattern()...
> 
> The problem is that hidden files are hidden, lsof can't traverse them
> normally.  It would be good to unhide them in some ways, and for me
> that would at least mean that you can
> 
>  1) query the path (proc/PID/fd/N link)
>  2) query fdinfo
>  3) query hidden files

Then by all means we can come up with a scheme in procfs that displays
this hierarchically if we have to.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Tue, 13 May 2025 at 09:39, Christian Brauner <brauner@kernel.org> wrote:

> No, the xattr interface is ugly as hell and I don't want it used as a
> generic information transportation information interface. And I don't
> want a single thing that sets a precedent in that direction.

You are getting emotional and the last messages from you contain zero
technical details.

I know about the buffer sizing one, can you describe your other gripes?

> > But if the data is inherently variable sized, adding specialized
> > interface is not going to magically solve that.
> >
> > Instead we can concentrate on solving the buffer sizing problem
> > generally, so that all may benefit.
>
> The xattr system call as far as I'm concerned is not going to be pimped
> to support stuff like that.

Heh?  IIRC there were positive reactions to e.g. "O_XATTR", it just
didn't get implemented.  Can try to dig this up from the archives.

> Then by all means we can come up with a scheme in procfs that displays
> this hierarchically if we have to.

Yeah, perhaps it's doable.

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Tue, 13 May 2025 at 09:57, Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 13 May 2025 at 09:39, Christian Brauner <brauner@kernel.org> wrote:

> > The xattr system call as far as I'm concerned is not going to be pimped
> > to support stuff like that.
>
> Heh?  IIRC there were positive reactions to e.g. "O_XATTR", it just
> didn't get implemented.  Can try to dig this up from the archives.

Here it is:

https://lore.kernel.org/all/CAHk-=wjzLmMRf=QG-n+1HnxWCx4KTQn9+OhVvUSJ=ZCQd6Y1WA@mail.gmail.com/

Quoting Linus inline:

| IOW, if you do something more along the lines of
|
|        fd = open(""foo/bar", O_PATH);
|        metadatafd = openat(fd, "metadataname", O_ALT);
|
| it might be workable.
|
| So you couldn't do it with _one_ pathname, because that is always
| fundamentally going to hit pathname lookup rules.
|
| But if you start a new path lookup with new rules, that's fine.
|
| This is what I think xattrs should always have done, because they are
| broken garbage.
|
| In fact, if we do it right, I think we could have "getxattr()" be 100%
| equivalent to (modulo all the error handling that this doesn't do, of
| course):
|
|   ssize_t getxattr(const char *path, const char *name,
|                         void *value, size_t size)
|   {
|      int fd, attrfd;
|
|      fd = open(path, O_PATH);
|      attrfd = openat(fd, name, O_ALT);
|      close(fd);
|      read(attrfd, value, size);
|      close(attrfd);
|   }
|
| and you'd still use getxattr() and friends as a shorthand (and for
| POSIX compatibility), but internally in the kernel we'd have a
| interface around that "xattrs are just file handles" model.
|
|                Linus

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Chen Linxuan]

On Tue, May 13, 2025 at 3:58 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 13 May 2025 at 09:39, Christian Brauner <brauner@kernel.org> wrote:
>
> > No, the xattr interface is ugly as hell and I don't want it used as a
> > generic information transportation information interface. And I don't
> > want a single thing that sets a precedent in that direction.
>
> You are getting emotional and the last messages from you contain zero
> technical details.
>
> I know about the buffer sizing one, can you describe your other gripes?
>
> > > But if the data is inherently variable sized, adding specialized
> > > interface is not going to magically solve that.
> > >
> > > Instead we can concentrate on solving the buffer sizing problem
> > > generally, so that all may benefit.
> >
> > The xattr system call as far as I'm concerned is not going to be pimped
> > to support stuff like that.
>
> Heh?  IIRC there were positive reactions to e.g. "O_XATTR", it just
> didn't get implemented.  Can try to dig this up from the archives.
>
> > Then by all means we can come up with a scheme in procfs that displays
> > this hierarchically if we have to.
>
> Yeah, perhaps it's doable.

In my opinion, adding relevant directories and nodes under procfs does not seem
to be much different from what I did in this patch by adding nodes
under /sys/fs/fuse.
This kind of solution would still be a somewhat “non-generic” approach.
For io_uring, scm_rights, and fuse backing files,
these newly added files or directories will eventually have their own
specific names.

I’m starting to wonder: is it really meaningful to pursue “genericity”
in this context?
Especially considering that io_uring already has its own “non-generic” handling.
For introducing a new way to expose kernel-held file resources,
would the maintainers of these two features even be willing to
coordinate and make changes?

Thanks,
Chen Linxuan

>
> Thanks,
> Miklos
>
>

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Fri, 20 Jun 2025 at 08:12, Chen Linxuan <chenlinxuan@uniontech.com> wrote:

> In my opinion, adding relevant directories and nodes under procfs does not seem
> to be much different from what I did in this patch by adding nodes
> under /sys/fs/fuse.
> This kind of solution would still be a somewhat “non-generic” approach.
> For io_uring, scm_rights, and fuse backing files,
> these newly added files or directories will eventually have their own
> specific names.

Why?  Name the attribute "hidden_files" and then it's generic.

The underlying problem is the overgrowth of fdinfo files.  It was
never meant to contain arrays, let alone hierarchies.

Thanks,
Miklos

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Amir Goldstein]

On Fri, May 9, 2025 at 4:43 PM Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> On Fri, May 9, 2025 at 10:00 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Fri, 9 May 2025 at 08:34, Chen Linxuan via B4 Relay
> > <devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
> > >
> > > From: Chen Linxuan <chenlinxuan@uniontech.com>
> > >
> > > Add a new FUSE control file "/sys/fs/fuse/connections/*/backing_files"
> > > that exposes the paths of all backing files currently being used in
> > > FUSE mount points. This is particularly valuable for tracking and
> > > debugging files used in FUSE passthrough mode.
> >
> > This is good work, thanks.
> >
> > My concern is that this is a very fuse specific interface, even though
> > the problem is more generic: list hidden open files belonging to a
> > kernel object, but not installed in any fd:
> >
> >  - SCM_RIGHTS
> >  - io_uring
>
> Note that io_uring has already exposed information about fixed files
> in its fdinfo.
>
> >  - fuse
> >
> > So we could have a new syscall or set of syscalls for this purpose.
> > But that again goes against my "this is not generic enough" pet peeve.
> >
> > So we had this idea of reusing getxattr and listxattr (or implementing
> > a new set of syscalls with the same signature) to allow retrieving a
> > hierarchical set of attributes belonging to a kernel object.  This one
> > would also fit that pattern, so...
> >
> > Thoughts?

I remember that there was some push back on this idea.
If there was no push back, you probably wouldn't have written
listmount/statmount...
so I am a bit concerned about sending Chen down this rabbit hole.

I think that for lsof, any way we present the information in fdinfo
that is parsable would be good enough for lsof to follow.

We could also list a full copy of backing_files table in fdinfo
of all the /dev/fuse open files, that will give lsof the pid of fuse server
in high likelihood.

But this is not very scalable with a large number of backing_files. hmm.

Is it a bad idea to merge the connections/N/backing_files code anyway
at least for debugging?

The extra fdinfo in patch 3 is just useful.
I don't see why we should not add it regardless of the standard  way
to iterate all backing_files.

Thanks,
Amir.

Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

[Miklos Szeredi]

On Fri, 9 May 2025 at 17:19, Amir Goldstein <amir73il@gmail.com> wrote:

> I remember that there was some push back on this idea.
> If there was no push back, you probably wouldn't have written
> listmount/statmount...

That's true.  IIRC one of arguments was "we don't want to parse
strings", which is not relevant here, since both proposals are text
based.

Not sure if there were any others, will dig into it.

>
> I think that for lsof, any way we present the information in fdinfo
> that is parsable would be good enough for lsof to follow.
>
> We could also list a full copy of backing_files table in fdinfo
> of all the /dev/fuse open files, that will give lsof the pid of fuse server
> in high likelihood.

Right.

> But this is not very scalable with a large number of backing_files. hmm.

That's one of the reasons why I'm advocating a hierarchical
alternative to the flat fdinfo file.

> Is it a bad idea to merge the connections/N/backing_files code anyway
> at least for debugging?

Maybe not, not sure.

> The extra fdinfo in patch 3 is just useful.
> I don't see why we should not add it regardless of the standard  way
> to iterate all backing_files.

Ah, missed 3/3, sorry.  Will review that shortly.

Thanks,
Miklos

[PATCH v3 3/3] fs: fuse: add more information to fdinfo

[Chen Linxuan via B4 Relay]

From: Chen Linxuan <chenlinxuan@uniontech.com>

This commit add fuse connection device id, open_flags and backing
files, to fdinfo of opened fuse files.

Related discussions can be found at links below.

Link: https://lore.kernel.org/all/CAOQ4uxgS3OUy9tpphAJKCQFRAn2zTERXXa0QN_KvP6ZOe2KVBw@mail.gmail.com/
Link: https://lore.kernel.org/all/CAOQ4uxgkg0uOuAWO2wOPNkMmD9wqd5wMX+gTfCT-zVHBC8CkZg@mail.gmail.com/
Cc: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
 fs/fuse/file.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 754378dd9f7159f20fde6376962d45c4c706b868..1e54965780e9d625918c22a3dea48ba5a9a5ed1b 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -8,6 +8,8 @@
 
 #include "fuse_i.h"
 
+#include "linux/idr.h"
+#include "linux/rcupdate.h"
 #include <linux/pagemap.h>
 #include <linux/slab.h>
 #include <linux/kernel.h>
@@ -3392,6 +3394,21 @@ static ssize_t fuse_copy_file_range(struct file *src_file, loff_t src_off,
 	return ret;
 }
 
+static void fuse_file_show_fdinfo(struct seq_file *seq, struct file *f)
+{
+	struct fuse_file *ff = f->private_data;
+	struct fuse_conn *fc = ff->fm->fc;
+	struct file *backing_file = fuse_file_passthrough(ff);
+
+	seq_printf(seq, "fuse conn:%u open_flags:%u\n", fc->dev, ff->open_flags);
+
+	if (backing_file) {
+		seq_puts(seq, "fuse backing_file: ");
+		seq_file_path(seq, backing_file, " \t\n\\");
+		seq_puts(seq, "\n");
+	}
+}
+
 static const struct file_operations fuse_file_operations = {
 	.llseek		= fuse_file_llseek,
 	.read_iter	= fuse_file_read_iter,
@@ -3411,6 +3428,9 @@ static const struct file_operations fuse_file_operations = {
 	.poll		= fuse_file_poll,
 	.fallocate	= fuse_file_fallocate,
 	.copy_file_range = fuse_copy_file_range,
+#ifdef CONFIG_PROC_FS
+	.show_fdinfo	= fuse_file_show_fdinfo,
+#endif
 };
 
 static const struct address_space_operations fuse_file_aops  = {

-- 
2.43.0

Re: [PATCH v3 3/3] fs: fuse: add more information to fdinfo

[Chen Linxuan]

On Fri, May 9, 2025 at 2:34 PM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@uniontech.com>
>
> This commit add fuse connection device id, open_flags and backing
> files, to fdinfo of opened fuse files.
>
> Related discussions can be found at links below.
>
> Link: https://lore.kernel.org/all/CAOQ4uxgS3OUy9tpphAJKCQFRAn2zTERXXa0QN_KvP6ZOe2KVBw@mail.gmail.com/
> Link: https://lore.kernel.org/all/CAOQ4uxgkg0uOuAWO2wOPNkMmD9wqd5wMX+gTfCT-zVHBC8CkZg@mail.gmail.com/
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---
>  fs/fuse/file.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 754378dd9f7159f20fde6376962d45c4c706b868..1e54965780e9d625918c22a3dea48ba5a9a5ed1b 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -8,6 +8,8 @@
>
>  #include "fuse_i.h"
>
> +#include "linux/idr.h"
> +#include "linux/rcupdate.h"
>  #include <linux/pagemap.h>
>  #include <linux/slab.h>
>  #include <linux/kernel.h>
> @@ -3392,6 +3394,21 @@ static ssize_t fuse_copy_file_range(struct file *src_file, loff_t src_off,
>         return ret;
>  }
>
> +static void fuse_file_show_fdinfo(struct seq_file *seq, struct file *f)
> +{
> +       struct fuse_file *ff = f->private_data;
> +       struct fuse_conn *fc = ff->fm->fc;
> +       struct file *backing_file = fuse_file_passthrough(ff);
> +
> +       seq_printf(seq, "fuse conn:%u open_flags:%u\n", fc->dev, ff->open_flags);

Note: The fc->dev is already accessible to userspace.
The mnt_id field in /proc/PID/fdinfo/FD references a mount,
which can be found in /proc/PID/mountinfo.

And this file includes the device ID.

> +
> +       if (backing_file) {
> +               seq_puts(seq, "fuse backing_file: ");
> +               seq_file_path(seq, backing_file, " \t\n\\");
> +               seq_puts(seq, "\n");
> +       }
> +}
> +
>  static const struct file_operations fuse_file_operations = {
>         .llseek         = fuse_file_llseek,
>         .read_iter      = fuse_file_read_iter,
> @@ -3411,6 +3428,9 @@ static const struct file_operations fuse_file_operations = {
>         .poll           = fuse_file_poll,
>         .fallocate      = fuse_file_fallocate,
>         .copy_file_range = fuse_copy_file_range,
> +#ifdef CONFIG_PROC_FS
> +       .show_fdinfo    = fuse_file_show_fdinfo,
> +#endif
>  };
>
>  static const struct address_space_operations fuse_file_aops  = {
>
> --
> 2.43.0
>
>
>
>

Re: [PATCH v3 3/3] fs: fuse: add more information to fdinfo

[Amir Goldstein]

On Fri, May 9, 2025 at 8:40 AM Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> On Fri, May 9, 2025 at 2:34 PM Chen Linxuan via B4 Relay
> <devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
> >
> > From: Chen Linxuan <chenlinxuan@uniontech.com>
> >
> > This commit add fuse connection device id, open_flags and backing
> > files, to fdinfo of opened fuse files.
> >
> > Related discussions can be found at links below.
> >
> > Link: https://lore.kernel.org/all/CAOQ4uxgS3OUy9tpphAJKCQFRAn2zTERXXa0QN_KvP6ZOe2KVBw@mail.gmail.com/
> > Link: https://lore.kernel.org/all/CAOQ4uxgkg0uOuAWO2wOPNkMmD9wqd5wMX+gTfCT-zVHBC8CkZg@mail.gmail.com/
> > Cc: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> > ---
> >  fs/fuse/file.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 754378dd9f7159f20fde6376962d45c4c706b868..1e54965780e9d625918c22a3dea48ba5a9a5ed1b 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -8,6 +8,8 @@
> >
> >  #include "fuse_i.h"
> >
> > +#include "linux/idr.h"
> > +#include "linux/rcupdate.h"

These are not needed.

> >  #include <linux/pagemap.h>
> >  #include <linux/slab.h>
> >  #include <linux/kernel.h>
> > @@ -3392,6 +3394,21 @@ static ssize_t fuse_copy_file_range(struct file *src_file, loff_t src_off,
> >         return ret;
> >  }
> >
> > +static void fuse_file_show_fdinfo(struct seq_file *seq, struct file *f)
> > +{
> > +       struct fuse_file *ff = f->private_data;
> > +       struct fuse_conn *fc = ff->fm->fc;
> > +       struct file *backing_file = fuse_file_passthrough(ff);
> > +
> > +       seq_printf(seq, "fuse conn:%u open_flags:%u\n", fc->dev, ff->open_flags);
>
> Note: The fc->dev is already accessible to userspace.
> The mnt_id field in /proc/PID/fdinfo/FD references a mount,
> which can be found in /proc/PID/mountinfo.
>
> And this file includes the device ID.
>

True, but the direct reference to conn here does not hurt.

> > +
> > +       if (backing_file) {
> > +               seq_puts(seq, "fuse backing_file: ");
> > +               seq_file_path(seq, backing_file, " \t\n\\");
> > +               seq_puts(seq, "\n");
> > +       }
> > +}
> > +

With unneeded includes fixed feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Please wait before posting v4 to give other developers and Miklos
a chance to review v3.

Thanks,
Amir.

Re: [PATCH v3 3/3] fs: fuse: add more information to fdinfo

[Miklos Szeredi]

On Fri, 9 May 2025 at 08:34, Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:

>  static const struct file_operations fuse_file_operations = {
>         .llseek         = fuse_file_llseek,
>         .read_iter      = fuse_file_read_iter,
> @@ -3411,6 +3428,9 @@ static const struct file_operations fuse_file_operations = {
>         .poll           = fuse_file_poll,
>         .fallocate      = fuse_file_fallocate,
>         .copy_file_range = fuse_copy_file_range,
> +#ifdef CONFIG_PROC_FS
> +       .show_fdinfo    = fuse_file_show_fdinfo,
> +#endif
>  };

The backing file mechanism is an internal implementation detail of
fuse and does not need to be displayed in the fuse file's fdinfo.
Currently we can only have one backing file per fuse file, but that
may well change in the future, as well as other details like offset
into the backing file (think FS_IOC_FIEMAP).

So NAK unless there's a very good use case for this.

Adding fdinfo for the /dev/fuse file is encouraged, it would be good
to be able to retrieve the connection number from the dev fd.

Thanks,
Miklos

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Amir Goldstein]

On Fri, May 9, 2025 at 8:34 AM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@kernel.org> wrote:
>
> Please review this patch series carefully. I am new to kernel
> development and I am not quite sure if I have followed the best
> practices, especially in terms of seq_file, error handling and locking.
> I would appreciate any feedback.
>
> I have do some simply testing using libfuse example [1]. It seems to
> work well.
>
> [1]: https://github.com/libfuse/libfuse/blob/master/example/passthrough_hp.cc
>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---

Very nice work Chen!

Please give reviewers some more time to review before posting v4
and you do not have to post v4 on account of my minor comments
unless Miklos requests it.

One thing that now comes to mind is how lsof is going to use and present
this information?

I did not look at how lsof presents io_uring fixed files, but I assume that
an io_uring is always associated with a specific user process.
How does lsof display the io_uring files?

I guess with io_uring, lsof can list a pid which admin can kill to release the
open files?
This is not the case with displaying conn, because lsof is not designed
to list fuse conn.

Is there a way for userspace to get from conn to fuse server pid?

Even if there were a way, I think that the fuse connection and open
backing files can certainly outlive the killed fuse server process.

But still, in the common case, if lsof can blame the fuse server process,
in most cases, killing the fuse server process will release the backing files,
so it may be worth looking into this.

Thanks,
Amir.

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Miklos Szeredi]

On Fri, 9 May 2025 at 09:36, Amir Goldstein <amir73il@gmail.com> wrote:

> This is not the case with displaying conn, because lsof is not designed
> to list fuse conn.
>
> Is there a way for userspace to get from conn to fuse server pid?

Define "server pid".

One such definition would be

 "A fuse server is a process that has an open file descriptor
referring to a /dev/fuse instance."

This definition allows a mapping between tasks and fuse connections to
be established.  Note that this is not a 1:1 mapping.  Multiple
processes could have the same fuse fd (or a clone) open and one
process could have multiple different connections associated with it.

This might be sufficient for lsof if it can find out the connection
number from the fd.  E.g. adding "fuse_connection: N" to fdinfo would
work, I think.

Thanks,
Miklos

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Joanne Koong]

On Fri, May 9, 2025 at 6:23 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, 9 May 2025 at 09:36, Amir Goldstein <amir73il@gmail.com> wrote:
>
> > This is not the case with displaying conn, because lsof is not designed
> > to list fuse conn.
> >
> > Is there a way for userspace to get from conn to fuse server pid?
>
> Define "server pid".
>
> One such definition would be
>
>  "A fuse server is a process that has an open file descriptor
> referring to a /dev/fuse instance."
>
> This definition allows a mapping between tasks and fuse connections to
> be established.  Note that this is not a 1:1 mapping.  Multiple
> processes could have the same fuse fd (or a clone) open and one
> process could have multiple different connections associated with it.
>
> This might be sufficient for lsof if it can find out the connection
> number from the fd.  E.g. adding "fuse_connection: N" to fdinfo would
> work, I think.

For getting from conn to fuse server pid, what about adding the server
pid to fuse's sysfs info? This use case has come up a few times in
production where we've encountered a stuck server and have wanted to
identify its pid. I have a patch on a branch I need to clean up and
send out for this, but it adds a new "info" file to
/sys/fs/fuse/connections/*/ where libfuse can write any identifying
info to that file like the server pid or name. If the connection gets
migrated to another process then libfuse is responsible for modifying
that to reflect the correct info.

Thanks,
Joanne

>
> Thanks,
> Miklos
>

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Miklos Szeredi]

On Tue, 13 May 2025 at 20:52, Joanne Koong <joannelkoong@gmail.com> wrote:

> For getting from conn to fuse server pid, what about adding the server
> pid to fuse's sysfs info? This use case has come up a few times in
> production where we've encountered a stuck server and have wanted to
> identify its pid. I have a patch on a branch I need to clean up and
> send out for this, but it adds a new "info" file to
> /sys/fs/fuse/connections/*/ where libfuse can write any identifying
> info to that file like the server pid or name. If the connection gets
> migrated to another process then libfuse is responsible for modifying
> that to reflect the correct info.

Fine, but then why not just write something in /var/run/fuse?

Thanks,
Miklos

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Joanne Koong]

On Wed, May 14, 2025 at 1:50 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 13 May 2025 at 20:52, Joanne Koong <joannelkoong@gmail.com> wrote:
>
> > For getting from conn to fuse server pid, what about adding the server
> > pid to fuse's sysfs info? This use case has come up a few times in
> > production where we've encountered a stuck server and have wanted to
> > identify its pid. I have a patch on a branch I need to clean up and
> > send out for this, but it adds a new "info" file to
> > /sys/fs/fuse/connections/*/ where libfuse can write any identifying
> > info to that file like the server pid or name. If the connection gets
> > migrated to another process then libfuse is responsible for modifying
> > that to reflect the correct info.
>
> Fine, but then why not just write something in /var/run/fuse?

Oh cool, I didn't know there's a /var/run directory. But I guess one
advantage of doing it in sysfs is that it'll work for unprivileged
servers whereas I think with /var/run/, there needs to be elevated
permissions to write to anything in that directory path.


Thanks,
Joanne

>
> Thanks,
> Miklos

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Chen Linxuan]

On Thu, May 15, 2025 at 7:04 AM Joanne Koong <joannelkoong@gmail.com> wrote:

> But I guess one advantage of doing it in sysfs is that it'll work for
> unprivileged servers whereas I think with /var/run/, there needs
> to be elevated permissions to write to anything in that directory path.

I suggest something like $XDG_RUNTIME_DIR/libfuse for unprivileged servers.
It usually is something like /var/run/user/1000/libfuse.

Thanks,
Chen Linxuan

Re: [PATCH v3 0/3] fuse: Expose more information of fuse backing files to userspace

[Joanne Koong]

On Wed, May 14, 2025 at 8:17 PM Chen Linxuan <chenlinxuan@uniontech.com> wrote:
>
> On Thu, May 15, 2025 at 7:04 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> > But I guess one advantage of doing it in sysfs is that it'll work for
> > unprivileged servers whereas I think with /var/run/, there needs
> > to be elevated permissions to write to anything in that directory path.
>
> I suggest something like $XDG_RUNTIME_DIR/libfuse for unprivileged servers.
> It usually is something like /var/run/user/1000/libfuse.

Cool, I think this works then.

Thanks,
Joanne
>
> Thanks,
> Chen Linxuan