On Wed, May 7, 2025 at 5:29 AM Chen Linxuan <chenlinxuan@xxxxxxxxxxxxx> wrote: > > 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: > > Link: https://lore.kernel.org/all/4b64a41c-6167-4c02-8bae-3021270ca519@xxxxxxxxxxx/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@xxxxxxxxxxxxxx/ > remove newline > Cc: Amir Goldstein <amir73il@xxxxxxxxx> > Signed-off-by: Chen Linxuan <chenlinxuan@xxxxxxxxxxxxx> > > --- > Please review this patch 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. Very nice work! > > I have do some simply testing using libfuse example [1]. It seems to > work well. It would be great if you could add basic sanity tests to libfuse maybe in test_passthrough_hp(), but I do not see any tests for /sys/fs/fuse/connections. I also see that there is one kernel selftest that mounts a fuse fs tools/testing/selftests/memfd maybe that is an easier way to write a simple test to verify the /sys/fs/fuse/connections functionally. Anyway, I do not require that you do that as a condition for merging this patch, but I may require that for removing CAP_SYS_ADMIN ;) > > [1]: https://github.com/libfuse/libfuse/blob/master/example/passthrough_hp.cc > --- > fs/fuse/control.c | 129 +++++++++++++++++++++++++++++++++++++++++++++- > fs/fuse/fuse_i.h | 2 +- > 2 files changed, 129 insertions(+), 2 deletions(-) > > diff --git a/fs/fuse/control.c b/fs/fuse/control.c > index 2a730d88cc3bd..4d1e0acc5030f 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,129 @@ static ssize_t fuse_conn_congestion_threshold_write(struct file *file, > return ret; > } > > +struct fuse_backing_files_seq_state { > + struct fuse_conn *fc; > + int pos; It will be more clear to call this 'backing_id'. It is more than an abstract pos in this context. > +}; > + > +static void *fuse_backing_files_seq_start(struct seq_file *seq, loff_t *pos) > +{ > + struct fuse_backing_files_seq_state *state = seq->private; > + struct fuse_conn *fc = state->fc; > + > + if (!fc) > + return NULL; > + > + spin_lock(&fc->lock); > + > + if (*pos > idr_get_cursor(&fc->backing_files_map)) { This won't do after the ida allocator has wrapped up back to 1, it will not iterate the high ids. Please look at using idr_get_next() iteration, like bpf_prog_seq_ops. With that change, I don't think that you need to take the spin lock for iteration. I think that you can use rcu_read_lock() for the scope of each start(). next(), show() because we do not need to promise a "snapshot" of the backing_file at a specific time. If backing files are added/removed while iterating it is undefined if they are listed or not, just like readdir. > + spin_unlock(&fc->lock); > + return NULL; Not critical, but if you end up needing a "scoped" unlock for the entire iteration, you can use the unlock in stop() if you return ERR_PTR(ENOENT) instead of NULL in those error conditions. > + } > + > + state->pos = *pos; > + return state; > +} > + > +static void *fuse_backing_files_seq_next(struct seq_file *seq, void *v, > + loff_t *pos) > +{ > + struct fuse_backing_files_seq_state *state = seq->private; > + > + (*pos)++; > + state->pos = *pos; > + > + if (state->pos > idr_get_cursor(&state->fc->backing_files_map)) { > + spin_unlock(&state->fc->lock); > + return NULL; > + } > + > + return state; > +} > + > +static int fuse_backing_files_seq_show(struct seq_file *seq, void *v) > +{ > + struct fuse_backing_files_seq_state *state = seq->private; > + struct fuse_conn *fc = state->fc; > + struct fuse_backing *fb; > + > + fb = idr_find(&fc->backing_files_map, state->pos); You must fuse_backing_get/put(fb) around dereferencing fb->file if not holding the fc->lock. See fuse_passthrough_open(). > + if (!fb || !fb->file) > + return 0; > + > + seq_file_path(seq, fb->file, " \t\n\\"); Pls print the backing id that is associated with the open file. I wonder out loud if we should also augment the backing fd information in fdinfo of specific open fuse FOPEN_PASSTHROUGH files? I am not requiring that you do that, but if you want to take a look I think that it could be cool to display this info, along with open_flags and other useful fuse_file information. Thanks, Amir. > + seq_puts(seq, "\n"); > + > + return 0; > +} > + > +static void fuse_backing_files_seq_stop(struct seq_file *seq, void *v) > +{ > + struct fuse_backing_files_seq_state *state = seq->private; > + > + if (v) > + spin_unlock(&state->fc->lock); > +} > + > +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) > +{ > + struct fuse_conn *fc; > + struct fuse_backing_files_seq_state *state; > + int err; > + > + fc = fuse_ctl_file_conn_get(file); > + if (!fc) > + return -ENOTCONN; > + > + err = seq_open(file, &fuse_backing_files_seq_ops); > + if (err) { > + fuse_conn_put(fc); > + return err; > + } > + > + state = kmalloc(sizeof(*state), GFP_KERNEL); > + if (!state) { > + seq_release(file->f_inode, file); > + fuse_conn_put(fc); > + return -ENOMEM; > + } > + > + state->fc = fc; > + state->pos = 0; > + ((struct seq_file *)file->private_data)->private = state; > + > + return 0; > +} > + > +static int fuse_backing_files_seq_release(struct inode *inode, > + struct file *file) > +{ > + struct seq_file *seq = file->private_data; > + struct fuse_backing_files_seq_state *state = seq->private; > + > + if (state) { > + fuse_conn_put(state->fc); > + kfree(state); > + seq->private = NULL; > + } > + > + return seq_release(inode, file); > +} > + > +static const struct file_operations fuse_conn_passthrough_backing_files_ops = { > + .open = fuse_backing_files_seq_open, > + .read = seq_read, > + .llseek = seq_lseek, > + .release = fuse_backing_files_seq_release, > +}; > + > static const struct file_operations fuse_ctl_abort_ops = { > .open = nonseekable_open, > .write = fuse_conn_abort_write, > @@ -270,7 +394,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_passthrough_backing_files_ops)) > goto err; > > return 0; > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index d56d4fd956db9..2830b05bb0928 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 >
