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

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

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

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.

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

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

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.

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

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.

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

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.

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

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

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

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

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.

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

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

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,
> +};
> +

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.

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.

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.