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

Chen Linxuan via B4 Relay posted 3 patches 9 months ago
There is a newer version of this series
[PATCH v2 2/3] fs: fuse: add backing_files control file
Posted by Chen Linxuan via B4 Relay 9 months ago
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
Posted by Amir Goldstein 9 months ago
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
Posted by Chen Linxuan 9 months ago
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.