On Thu, May 8, 2025 at 10:54 AM Chen Linxuan via B4 Relay <devnull+chenlinxuan.uniontech.com@xxxxxxxxxx> wrote: > > From: Chen Linxuan <chenlinxuan@xxxxxxxxxxxxx> > > 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@xxxxxxxxxxx/T/#mc73e04df56b8830b1d7b06b5d9f22e594fba423e > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhAY1m7ubJ3p-A3rSufw_53WuDRMT1Zqe_OC0bP_Fb3Zw@xxxxxxxxxxxxxx/ > Cc: Amir Goldstein <amir73il@xxxxxxxxx> > Signed-off-by: Chen Linxuan <chenlinxuan@xxxxxxxxxxxxx> > --- > 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.
