Re: [PATCH 3/5] fuse: move the passthrough-specific code back to passthrough.c

Amir Goldstein
2025-09-17T02:47:33Z

On Tue, Sep 16, 2025 at 2:27 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> From: Darrick J. Wong <djwong@kernel.org>
>
> In preparation for iomap, move the passthrough-specific validation code
> back to passthrough.c and create a new Kconfig item for conditional
> compilation of backing.c. In the next patch, iomap will share the
> backing structures.
>
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/fuse/fuse_i.h | 23 +++++++++--
> include/uapi/linux/fuse.h | 8 +++-
> fs/fuse/Kconfig | 4 ++
> fs/fuse/Makefile | 3 +
> fs/fuse/backing.c | 95 ++++++++++++++++++++++++++++++++++-----------
> fs/fuse/dev.c | 4 +-
> fs/fuse/inode.c | 4 +-
> fs/fuse/passthrough.c | 37 +++++++++++++++++-
> 8 files changed, 144 insertions(+), 34 deletions(-)
>
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 52db609e63eb54..4560687d619d76 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -96,10 +96,21 @@ struct fuse_submount_lookup {
> struct fuse_forget_link *forget;
> };
>
> +struct fuse_conn;
> +
> +/** Operations for subsystems that want to use a backing file */
> +struct fuse_backing_ops {
> + int (*may_admin)(struct fuse_conn *fc, uint32_t flags);
> + int (*may_open)(struct fuse_conn *fc, struct file *file);
> + int (*may_close)(struct fuse_conn *fc, struct file *file);
> + unsigned int type;
> +};
> +
> /** Container for data related to mapping to backing file */
> struct fuse_backing {
> struct file *file;
> struct cred *cred;
> + const struct fuse_backing_ops *ops;

Please argue why we need a mix of passthrough backing
files and iomap backing bdev on the same filesystem.

Same as my argument against passthrough/iomap on
same fuse_backing:
If you do not plan to test it, and nobody asked for it, please do
not allow it - it's bad for code test coverage.

I think at this point in time FUSE_PASSTHROUGH and
FUSE_IOMAP should be mutually exclusive and
fuse_backing_ops could be set at fc level.
If we want to move them for per fuse_backing later
we can always do that when the use cases and tests arrive.

Thanks,
Amir.

>
> /** refcount */
> refcount_t count;
> @@ -968,7 +979,7 @@ struct fuse_conn {
> /* New writepages go into this bucket */
> struct fuse_sync_bucket __rcu *curr_bucket;
>
> -#ifdef CONFIG_FUSE_PASSTHROUGH
> +#ifdef CONFIG_FUSE_BACKING
> /** IDR for backing files ids */
> struct idr backing_files_map;
> #endif
> @@ -1571,10 +1582,12 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
> unsigned int open_flags, fl_owner_t id, bool isdir);
>
> /* backing.c */
> -#ifdef CONFIG_FUSE_PASSTHROUGH
> +#ifdef CONFIG_FUSE_BACKING
> struct fuse_backing *fuse_backing_get(struct fuse_backing *fb);
> void fuse_backing_put(struct fuse_backing *fb);
> -struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id);
> +struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc,
> + const struct fuse_backing_ops *ops,
> + int backing_id);
> #else
>
> static inline struct fuse_backing *fuse_backing_get(struct fuse_backing *fb)
> @@ -1631,6 +1644,10 @@ static inline struct file *fuse_file_passthrough(struct fuse_file *ff)
> #endif
> }
>
> +#ifdef CONFIG_FUSE_PASSTHROUGH
> +extern const struct fuse_backing_ops fuse_passthrough_backing_ops;
> +#endif
> +
> ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *iter);
> ssize_t fuse_passthrough_write_iter(struct kiocb *iocb, struct iov_iter *iter);
> ssize_t fuse_passthrough_splice_read(struct file *in, loff_t *ppos,
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 1d76d0332f46f6..31b80f93211b81 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -1114,9 +1114,15 @@ struct fuse_notify_retrieve_in {
> uint64_t dummy4;
> };
>
> +#define FUSE_BACKING_TYPE_MASK (0xFF)
> +#define FUSE_BACKING_TYPE_PASSTHROUGH (0)
> +#define FUSE_BACKING_MAX_TYPE (FUSE_BACKING_TYPE_PASSTHROUGH)
> +
> +#define FUSE_BACKING_FLAGS_ALL (FUSE_BACKING_TYPE_MASK)
> +
> struct fuse_backing_map {
> int32_t fd;
> - uint32_t flags;
> + uint32_t flags; /* FUSE_BACKING_* */
> uint64_t padding;
> };
>
> diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> index a774166264de69..9563fa5387a241 100644
> --- a/fs/fuse/Kconfig
> +++ b/fs/fuse/Kconfig
> @@ -59,12 +59,16 @@ config FUSE_PASSTHROUGH
> default y
> depends on FUSE_FS
> select FS_STACK
> + select FUSE_BACKING
> help
> This allows bypassing FUSE server by mapping specific FUSE operations
> to be performed directly on a backing file.
>
> If you want to allow passthrough operations, answer Y.
>
> +config FUSE_BACKING
> + bool
> +
> config FUSE_IO_URING
> bool "FUSE communication over io-uring"
> default y
> diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> index 8ddd8f0b204ee5..36be6d715b111a 100644
> --- a/fs/fuse/Makefile
> +++ b/fs/fuse/Makefile
> @@ -13,7 +13,8 @@ obj-$(CONFIG_VIRTIO_FS) += virtiofs.o
> fuse-y := dev.o dir.o file.o inode.o control.o xattr.o acl.o readdir.o ioctl.o
> fuse-y += iomode.o
> fuse-$(CONFIG_FUSE_DAX) += dax.o
> -fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o backing.o
> +fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o
> +fuse-$(CONFIG_FUSE_BACKING) += backing.o
> fuse-$(CONFIG_SYSCTL) += sysctl.o
> fuse-$(CONFIG_FUSE_IO_URING) += dev_uring.o
>
> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> index 4afda419dd1416..da0dff288396ed 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
> @@ -6,6 +6,7 @@
> */
>
> #include "fuse_i.h"
> +#include "fuse_trace.h"
>
> #include <linux/file.h>
>
> @@ -69,32 +70,53 @@ static int fuse_backing_id_free(int id, void *p, void *data)
> struct fuse_backing *fb = p;
>
> WARN_ON_ONCE(refcount_read(&fb->count) != 1);
> +
> fuse_backing_free(fb);
> return 0;
> }
>
> void fuse_backing_files_free(struct fuse_conn *fc)
> {
> - idr_for_each(&fc->backing_files_map, fuse_backing_id_free, NULL);
> + idr_for_each(&fc->backing_files_map, fuse_backing_id_free, fc);
> idr_destroy(&fc->backing_files_map);
> }
>
> +static inline const struct fuse_backing_ops *
> +fuse_backing_ops_from_map(const struct fuse_backing_map *map)
> +{
> + switch (map->flags & FUSE_BACKING_TYPE_MASK) {
> +#ifdef CONFIG_FUSE_PASSTHROUGH
> + case FUSE_BACKING_TYPE_PASSTHROUGH:
> + return &fuse_passthrough_backing_ops;
> +#endif
> + default:
> + break;
> + }
> +
> + return NULL;
> +}
> +
> int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
> {
> struct file *file;
> - struct super_block *backing_sb;
> struct fuse_backing *fb = NULL;
> + const struct fuse_backing_ops *ops = fuse_backing_ops_from_map(map);
> + uint32_t op_flags = map->flags & ~FUSE_BACKING_TYPE_MASK;
> int res;
>
> pr_debug("%s: fd=%d flags=0x%x\n", __func__, map->fd, map->flags);
>
> - /* TODO: relax CAP_SYS_ADMIN once backing files are visible to lsof */
> - res = -EPERM;
> - if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> + res = -EOPNOTSUPP;
> + if (!ops)
> + goto out;
> + WARN_ON(ops->type != (map->flags & FUSE_BACKING_TYPE_MASK));
> +
> + res = ops->may_admin ? ops->may_admin(fc, op_flags) : 0;
> + if (res)
> goto out;
>
> res = -EINVAL;
> - if (map->flags || map->padding)
> + if (map->padding)
> goto out;
>
> file = fget_raw(map->fd);
> @@ -102,14 +124,8 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
> if (!file)
> goto out;
>
> - /* read/write/splice/mmap passthrough only relevant for regular files */
> - res = d_is_dir(file->f_path.dentry) ? -EISDIR : -EINVAL;
> - if (!d_is_reg(file->f_path.dentry))
> - goto out_fput;
> -
> - backing_sb = file_inode(file)->i_sb;
> - res = -ELOOP;
> - if (backing_sb->s_stack_depth >= fc->max_stack_depth)
> + res = ops->may_open ? ops->may_open(fc, file) : 0;
> + if (res)
> goto out_fput;
>
> fb = kmalloc(sizeof(struct fuse_backing), GFP_KERNEL);
> @@ -119,14 +135,15 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
>
> fb->file = file;
> fb->cred = prepare_creds();
> + fb->ops = ops;
> refcount_set(&fb->count, 1);
>
> res = fuse_backing_id_alloc(fc, fb);
> if (res < 0) {
> fuse_backing_free(fb);
> fb = NULL;
> + goto out;
> }
> -
> out:
> pr_debug("%s: fb=0x%p, ret=%i\n", __func__, fb, res);
>
> @@ -137,41 +154,71 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
> goto out;
> }
>
> +static struct fuse_backing *__fuse_backing_lookup(struct fuse_conn *fc,
> + int backing_id)
> +{
> + struct fuse_backing *fb;
> +
> + rcu_read_lock();
> + fb = idr_find(&fc->backing_files_map, backing_id);
> + fb = fuse_backing_get(fb);
> + rcu_read_unlock();
> +
> + return fb;
> +}
> +
> int fuse_backing_close(struct fuse_conn *fc, int backing_id)
> {
> - struct fuse_backing *fb = NULL;
> + struct fuse_backing *fb, *test_fb;
> + const struct fuse_backing_ops *ops;
> int err;
>
> pr_debug("%s: backing_id=%d\n", __func__, backing_id);
>
> - /* TODO: relax CAP_SYS_ADMIN once backing files are visible to lsof */
> - err = -EPERM;
> - if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> - goto out;
> -
> err = -EINVAL;
> if (backing_id <= 0)
> goto out;
>
> err = -ENOENT;
> - fb = fuse_backing_id_remove(fc, backing_id);
> + fb = __fuse_backing_lookup(fc, backing_id);
> if (!fb)
> goto out;
> + ops = fb->ops;
>
> - fuse_backing_put(fb);
> + err = ops->may_admin ? ops->may_admin(fc, 0) : 0;
> + if (err)
> + goto out_fb;
> +
> + err = ops->may_close ? ops->may_close(fc, fb->file) : 0;
> + if (err)
> + goto out_fb;
> +
> + err = -ENOENT;
> + test_fb = fuse_backing_id_remove(fc, backing_id);
> + if (!test_fb)
> + goto out_fb;
> +
> + WARN_ON(fb != test_fb);
> err = 0;
> + fuse_backing_put(test_fb);
> +out_fb:
> + fuse_backing_put(fb);
> out:
> pr_debug("%s: fb=0x%p, err=%i\n", __func__, fb, err);
>
> return err;
> }
>
> -struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id)
> +struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc,
> + const struct fuse_backing_ops *ops,
> + int backing_id)
> {
> struct fuse_backing *fb;
>
> rcu_read_lock();
> fb = idr_find(&fc->backing_files_map, backing_id);
> + if (fb && fb->ops != ops)
> + fb = NULL;
> fb = fuse_backing_get(fb);
> rcu_read_unlock();
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index e5aaf0c668bc11..281bc81f3b448b 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2654,7 +2654,7 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
> if (IS_ERR(fud))
> return PTR_ERR(fud);
>
> - if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> + if (!IS_ENABLED(CONFIG_FUSE_BACKING))
> return -EOPNOTSUPP;
>
> if (copy_from_user(&map, argp, sizeof(map)))
> @@ -2671,7 +2671,7 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> if (IS_ERR(fud))
> return PTR_ERR(fud);
>
> - if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> + if (!IS_ENABLED(CONFIG_FUSE_BACKING))
> return -EOPNOTSUPP;
>
> if (get_user(backing_id, argp))
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 14c35ce12b87d6..1e7298b2b89b58 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -995,7 +995,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
> fc->name_max = FUSE_NAME_LOW_MAX;
> fc->timeout.req_timeout = 0;
>
> - if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> + if (IS_ENABLED(CONFIG_FUSE_BACKING))
> fuse_backing_files_init(fc);
>
> INIT_LIST_HEAD(&fc->mounts);
> @@ -1032,7 +1032,7 @@ void fuse_conn_put(struct fuse_conn *fc)
> WARN_ON(atomic_read(&bucket->count) != 1);
> kfree(bucket);
> }
> - if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> + if (IS_ENABLED(CONFIG_FUSE_BACKING))
> fuse_backing_files_free(fc);
> call_rcu(&fc->rcu, delayed_release);
> }
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index e0b8d885bc81f3..9792d7b12a775b 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -164,7 +164,7 @@ struct fuse_backing *fuse_passthrough_open(struct file *file,
> goto out;
>
> err = -ENOENT;
> - fb = fuse_backing_lookup(fc, backing_id);
> + fb = fuse_backing_lookup(fc, &fuse_passthrough_backing_ops, backing_id);
> if (!fb)
> goto out;
>
> @@ -197,3 +197,38 @@ void fuse_passthrough_release(struct fuse_file *ff, struct fuse_backing *fb)
> put_cred(ff->cred);
> ff->cred = NULL;
> }
> +
> +static int fuse_passthrough_may_admin(struct fuse_conn *fc, unsigned int flags)
> +{
> + /* TODO: relax CAP_SYS_ADMIN once backing files are visible to lsof */
> + if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + if (flags)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int fuse_passthrough_may_open(struct fuse_conn *fc, struct file *file)
> +{
> + struct super_block *backing_sb;
> + int res;
> +
> + /* read/write/splice/mmap passthrough only relevant for regular files */
> + res = d_is_dir(file->f_path.dentry) ? -EISDIR : -EINVAL;
> + if (!d_is_reg(file->f_path.dentry))
> + return res;
> +
> + backing_sb = file_inode(file)->i_sb;
> + if (backing_sb->s_stack_depth >= fc->max_stack_depth)
> + return -ELOOP;
> +
> + return 0;
> +}
> +
> +const struct fuse_backing_ops fuse_passthrough_backing_ops = {
> + .type = FUSE_BACKING_TYPE_PASSTHROUGH,
> + .may_admin = fuse_passthrough_may_admin,
> + .may_open = fuse_passthrough_may_open,
> +};
>
>