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

[PATCH v2 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 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 | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/fuse/file.c    |  34 ++++++++++++++
 fs/fuse/fuse_i.h  |   2 +-
 3 files changed, 171 insertions(+), 3 deletions(-)
---
base-commit: d76bb1ebb5587f66b0f8b8099bfbb44722bc08b3
change-id: 20250508-fusectl-backing-files-6810d290e798

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

[PATCH v2 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

[PATCH v2 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 | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/fuse/fuse_i.h  |   2 +-
 2 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/control.c b/fs/fuse/control.c
index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..d1ac934d7b8949577545ffd20535c68a9c4ef90f 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,136 @@ 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_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;
+
+	if (*pos + 1 > INT_MAX)
+		return ERR_PTR(-EINVAL);
+
+	backing_id = *pos + 1;
+
+	fc = fuse_ctl_file_conn_get(seq->file);
+	if (!fc)
+		return ERR_PTR(-ENOTCONN);
+
+	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;
+
+	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;
+
+	(*pos)++;
+	state->backing_id++;
+
+	rcu_read_lock();
+
+	fb = idr_get_next(&state->fc->backing_files_map, &state->backing_id);
+
+	rcu_read_unlock();
+
+	if (!fb)
+		return NULL;
+
+
+	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);
+	if (!fb)
+		goto out;
+
+	if (!fb->file)
+		goto out_put_fb;
+
+	seq_printf(seq, "%5u: ", state->backing_id);
+	seq_file_path(seq, fb->file, " \t\n\\");
+	seq_puts(seq, "\n");
+
+out_put_fb:
+	fuse_backing_put(fb);
+out:
+	rcu_read_unlock();
+	return 0;
+}
+
+static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v)
+{
+	struct fuse_backing_files_seq_state *state;
+
+	if (!v)
+		return;
+
+	state = v;
+	fuse_conn_put(state->fc);
+	kvfree(state);
+}
+
+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,
@@ -270,7 +401,10 @@ int fuse_ctl_add_conn(struct fuse_conn *fc)
 				 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;
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 v2 2/3] fs: fuse: add backing_files control file

[Amir Goldstein]

On Thu, May 8, 2025 at 10:54 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>
> ---
>  fs/fuse/control.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  fs/fuse/fuse_i.h  |   2 +-
>  2 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..d1ac934d7b8949577545ffd20535c68a9c4ef90f 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,136 @@ 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_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;
> +
> +       if (*pos + 1 > INT_MAX)
> +               return ERR_PTR(-EINVAL);
> +
> +       backing_id = *pos + 1;

I am not sure if this +1 is correct.
Please make sure that you test reading in several chunks
not only from pos 0 to make sure this is right.
Assuming that is how the code gets to call start() with non zero pos?

I think that backing_id should always be in sync with *pos for that to
work correctly.
"The next() function should set ``*pos`` to a value that start() can use
to find the new location in the sequence."

That means that we do not really need the backing_id in the state.
We can just have a local backing_id var that reads from *pos and
*pos is set to it at the end of start/next.

> +
> +       fc = fuse_ctl_file_conn_get(seq->file);
> +       if (!fc)
> +               return ERR_PTR(-ENOTCONN);
> +
> +       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;
> +
> +       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;
> +
> +       (*pos)++;
> +       state->backing_id++;

Need to check for INT_MAX overflow?

> +
> +       rcu_read_lock();
> +
> +       fb = idr_get_next(&state->fc->backing_files_map, &state->backing_id);
> +
> +       rcu_read_unlock();
> +
> +       if (!fb)
> +               return NULL;

I think that I gave you the wrong advice on v1 review.
IIUC, if you return NULL from next(), stop() will get a NULL v arg,
so I think you need to put fc and free state here as well.
Please verify that.

Perhaps a small helper fuse_backing_files_seq_state_free()
can help the code look cleaner, because you may also need it
in the int overflow case above?

> +
> +
> +       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();

should be here.
After you get a ref on fb, it is safe to deref fb without RCU
so no need for the goto cleanup labels.

> +       if (!fb)
> +               goto out;
> +
> +       if (!fb->file)
> +               goto out_put_fb;
> +

This would be nicer as
      if (!fb->file) {

to avoid the goto.

> +       seq_printf(seq, "%5u: ", state->backing_id);
> +       seq_file_path(seq, fb->file, " \t\n\\");
> +       seq_puts(seq, "\n");
> +
> +out_put_fb:
> +       fuse_backing_put(fb);
> +out:
> +       rcu_read_unlock();
> +       return 0;
> +}
> +
> +static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v)
> +{
> +       struct fuse_backing_files_seq_state *state;
> +
> +       if (!v)
> +               return;
> +
> +       state = v;
> +       fuse_conn_put(state->fc);
> +       kvfree(state);

That could become a helper
static void fuse_backing_files_seq_state_free(struct
fuse_backing_files_seq_state *state)

and seq_stop() could become:

if (v)
       fuse_backing_files_seq_state_free(v);


Thanks,
Amir.

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

[Chen Linxuan]

On Thu, May 8, 2025 at 6:45 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, May 8, 2025 at 10:54 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>
> > ---
> >  fs/fuse/control.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  fs/fuse/fuse_i.h  |   2 +-
> >  2 files changed, 136 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> > index f0874403b1f7c91571f38e4ae9f8cebe259f7dd1..d1ac934d7b8949577545ffd20535c68a9c4ef90f 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,136 @@ 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_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;
> > +
> > +       if (*pos + 1 > INT_MAX)
> > +               return ERR_PTR(-EINVAL);
> > +
> > +       backing_id = *pos + 1;
>
> I am not sure if this +1 is correct.
> Please make sure that you test reading in several chunks
> not only from pos 0 to make sure this is right.
> Assuming that is how the code gets to call start() with non zero pos?

I am not very certain.
In seq_file.c, we can see some processes like stop, allocating a new buffer,
and restarting at the original pos when the buffer is full.

Without adding this +1, this part of the code actually doesn't work correctly.
It outputs the backing_file with id=1 twice.
I think the reason for this issue is that I didn't update the value of
pos with backing_id after calling idr_get_next().

>
> I think that backing_id should always be in sync with *pos for that to
> work correctly.
> "The next() function should set ``*pos`` to a value that start() can use
> to find the new location in the sequence."
>
> That means that we do not really need the backing_id in the state.
> We can just have a local backing_id var that reads from *pos and
> *pos is set to it at the end of start/next.

I think we need to maintain pos in the state
because show() does not accept pos as a parameter.

>
> > +
> > +       fc = fuse_ctl_file_conn_get(seq->file);
> > +       if (!fc)
> > +               return ERR_PTR(-ENOTCONN);
> > +
> > +       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;
> > +
> > +       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;
> > +
> > +       (*pos)++;
> > +       state->backing_id++;
>
> Need to check for INT_MAX overflow?

It shouldn't be necessary. Before overflow occurs, idr_get_next() will
already return NULL.

>
> > +
> > +       rcu_read_lock();
> > +
> > +       fb = idr_get_next(&state->fc->backing_files_map, &state->backing_id);
> > +
> > +       rcu_read_unlock();
> > +
> > +       if (!fb)
> > +               return NULL;
>
> I think that I gave you the wrong advice on v1 review.
> IIUC, if you return NULL from next(), stop() will get a NULL v arg,
> so I think you need to put fc and free state here as well.
> Please verify that.
>
> Perhaps a small helper fuse_backing_files_seq_state_free()
> can help the code look cleaner, because you may also need it
> in the int overflow case above?
>
> > +
> > +
> > +       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();
>
> should be here.
> After you get a ref on fb, it is safe to deref fb without RCU
> so no need for the goto cleanup labels.
>
> > +       if (!fb)
> > +               goto out;
> > +
> > +       if (!fb->file)
> > +               goto out_put_fb;
> > +
>
> This would be nicer as
>       if (!fb->file) {
>
> to avoid the goto.

I guess what you meant to say was if (fb->file)?

>
> > +       seq_printf(seq, "%5u: ", state->backing_id);
> > +       seq_file_path(seq, fb->file, " \t\n\\");
> > +       seq_puts(seq, "\n");
> > +
> > +out_put_fb:
> > +       fuse_backing_put(fb);
> > +out:
> > +       rcu_read_unlock();
> > +       return 0;
> > +}
> > +
> > +static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v)
> > +{
> > +       struct fuse_backing_files_seq_state *state;
> > +
> > +       if (!v)
> > +               return;
> > +
> > +       state = v;
> > +       fuse_conn_put(state->fc);
> > +       kvfree(state);
>
> That could become a helper
> static void fuse_backing_files_seq_state_free(struct
> fuse_backing_files_seq_state *state)
>
> and seq_stop() could become:
>
> if (v)
>        fuse_backing_files_seq_state_free(v);
>
>
> Thanks,
> Amir.
>
>

I will send the next version of the patch later.

[PATCH v2 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 and backing_id, if any, 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/
Cc: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
---
1. I wonder if we should display fuse connection device id here.

2. I don't think using idr_for_each_entry is a good idea. But I failed
   to find another way to get backing_id of fuse_backing effectively.
---
 fs/fuse/file.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 754378dd9f7159f20fde6376962d45c4c706b868..5cfb806aa5cd22c57814168eb33de77c6f213da0 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,35 @@ 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 *m, struct file *f)
+{
+	struct fuse_file *ff = f->private_data;
+	struct fuse_conn *fc = ff->fm->fc;
+	struct fuse_inode *fi = get_fuse_inode(file_inode(f));
+
+	seq_printf(m, "fuse_conn:\t%u\n", fc->dev);
+
+#ifdef CONFIG_FUSE_PASSTHROUGH
+	struct fuse_backing *fb;
+	struct fuse_backing *backing;
+	int backing_id;
+
+	if (ff->open_flags & FOPEN_PASSTHROUGH) {
+		fb = fuse_inode_backing(fi);
+		if (fb) {
+			rcu_read_lock();
+			idr_for_each_entry(&fc->backing_files_map, backing, backing_id) {
+				if (backing == fb) {
+					seq_printf(m, "fuse_backing_id:\t%d\n", backing_id);
+					break;
+				}
+			}
+			rcu_read_unlock();
+		}
+	}
+#endif
+}
+
 static const struct file_operations fuse_file_operations = {
 	.llseek		= fuse_file_llseek,
 	.read_iter	= fuse_file_read_iter,
@@ -3411,6 +3442,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 v2 3/3] fs: fuse: add more information to fdinfo

[Amir Goldstein]

On Thu, May 8, 2025 at 10:54 AM 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 and backing_id, if any, 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/
> Cc: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Chen Linxuan <chenlinxuan@uniontech.com>
> ---
> 1. I wonder if we should display fuse connection device id here.
>
> 2. I don't think using idr_for_each_entry is a good idea. But I failed
>    to find another way to get backing_id of fuse_backing effectively.

Indeed.
The thing is that a fuse file could have passthough to backing file
without that backing file having a backing id at all.

1. server maps backing file to backing id N
2. server associates opened file F to backing file N
3. server unmaps backing id N, so now the backing file is "anonymous"

The backing file remains referenced by the kernel until file F is released.

> ---
>  fs/fuse/file.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 754378dd9f7159f20fde6376962d45c4c706b868..5cfb806aa5cd22c57814168eb33de77c6f213da0 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,35 @@ 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 *m, struct file *f)
> +{
> +       struct fuse_file *ff = f->private_data;
> +       struct fuse_conn *fc = ff->fm->fc;
> +       struct fuse_inode *fi = get_fuse_inode(file_inode(f));
> +
> +       seq_printf(m, "fuse_conn:\t%u\n", fc->dev);
> +

Let's follow pattern of fanotify_fdinfo() and add some useful information:

seq_printf(m, "fuse conn:%u open_flags:%u\n", fc->dev, ff->open_flags);

> +#ifdef CONFIG_FUSE_PASSTHROUGH
> +       struct fuse_backing *fb;
> +       struct fuse_backing *backing;
> +       int backing_id;

I did not know that it is allowed to define local vars in the middle
of a function in kernel code.
anyway, you shouldn't have to use ifdef here.
compiler will optimize away code inside:
if (fuse_file_passthrough(ff)) {

When CONFIG_FUSE_PASSTHROUGH is not defined.

> +
> +       if (ff->open_flags & FOPEN_PASSTHROUGH) {
> +               fb = fuse_inode_backing(fi);
> +               if (fb) {
> +                       rcu_read_lock();
> +                       idr_for_each_entry(&fc->backing_files_map, backing, backing_id) {
> +                               if (backing == fb) {
> +                                       seq_printf(m, "fuse_backing_id:\t%d\n", backing_id);
> +                                       break;
> +                               }
> +                       }
> +                       rcu_read_unlock();

We cannot display backing_id here, but we can print the fuse_file_passthrough()
file path and we already have a reference to that file, so no need for
any locks/RCU,
so this should work:

   struct fuse_file *ff = f->private_data;
   struct fuse_conn *fc = ff->fm->fc;
   struct file *backing_file = fuse_file_passthrough(ff);

   seq_printf(m, "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, fb->file, " \t\n\\");
       seq_puts(seq, "\n");
    }

We want a separate line for backing_file because:
1. There may be multiple backing files per fuse file is the future
(see famfs patches)
2. In that case, there will likely be file range mapping information
printed in this line

Thanks,
Amir.