Message-ID: <CAC1kPDM5cN9p-Ri1WUEWt6JNiTZukekJyihYRT=qTwawVT3bFA@mail.gmail.com>
Date: Fri, 9 May 2025 15:39:26 +0800
From: Chen Linxuan <chenlinxuan@...ontech.com>
To: chenlinxuan@...ontech.com
Cc: Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] fs: fuse: add backing_files control file

On Fri, May 9, 2025 at 2:34 PM Chen Linxuan via B4 Relay
<devnull+chenlinxuan.uniontech.com@...nel.org> wrote:
>
> From: Chen Linxuan <chenlinxuan@...ontech.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@...il.com>
> Signed-off-by: Chen Linxuan <chenlinxuan@...ontech.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,
> +};
> +

Powered by blists - more mailing lists