| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAOQ4uxiWK5dNMkrriApMVZQi6apmnMijcCw5j4fa2thHFdnFcw@mail.gmail.com>
Date: Sat, 12 Sep 2020 14:06:02 +0300
From: Amir Goldstein <amir73il@...il.com>
To: Alessio Balsini <balsini@...roid.com>
Cc: Miklos Szeredi <miklos@...redi.hu>,
Akilesh Kailash <akailash@...gle.com>,
David Anderson <dvander@...gle.com>,
Eric Yan <eric.yan@...plus.com>, Jann Horn <jannh@...gle.com>,
Jens Axboe <axboe@...nel.dk>,
Martijn Coenen <maco@...roid.com>,
Palmer Dabbelt <palmer@...belt.com>,
Paul Lawrence <paullawrence@...gle.com>,
Stefano Duo <stefanoduo@...gle.com>,
Zimuzo Ezeozue <zezeozue@...gle.com>,
fuse-devel <fuse-devel@...ts.sourceforge.net>,
kernel-team <kernel-team@...roid.com>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V8 1/3] fuse: Definitions and ioctl() for passthrough
On Fri, Sep 11, 2020 at 7:34 PM Alessio Balsini <balsini@...roid.com> wrote:
>
> Introduce the new FUSE passthrough ioctl(), which allows userspace to
> specify a direct connection between a FUSE file and a lower file system
> file.
> Such ioctl() requires userspace to specify:
> - the file descriptor of one of its opened files,
> - the unique identifier of the FUSE request associated with a pending
> open/create operation,
> both encapsulated into a fuse_passthrough_out data structure.
> The ioctl() will search for the pending FUSE request matching the unique
> identifier, and update the passthrough file pointer of the request with the
> file pointer referenced by the passed file descriptor.
> When that pending FUSE request is handled, the passthrough file pointer
> is copied to the fuse_file data structure, so that the link between FUSE
> and lower file system is consolidated.
>
> In order for the passthrough mode to be successfully activated, the lower
> file system file must implement both read_ and write_iter file operations.
> This extra check avoids special pseudofiles to be targets for this feature.
> An additional enforced limitation is that when FUSE passthrough is enabled,
> no further file system stacking is allowed.
>
> Signed-off-by: Alessio Balsini <balsini@...roid.com>
> ---
[...]
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index bba747520e9b..eb223130a917 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -965,6 +965,12 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
> min_t(unsigned int, FUSE_MAX_MAX_PAGES,
> max_t(unsigned int, arg->max_pages, 1));
> }
> + if (arg->flags & FUSE_PASSTHROUGH) {
> + fc->passthrough = 1;
> + /* Prevent further stacking */
> + fc->sb->s_stack_depth =
> + FILESYSTEM_MAX_STACK_DEPTH;
> + }
That seems a bit limiting.
I suppose what you really want to avoid is loops into FUSE fd.
There may be a way to do this with forbidding overlay over FUSE passthrough
or the other way around.
You can set fc->sb->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH - 1
here and in passthrough ioctl you can check for looping into a fuse fs with
passthrough enabled on the passed fd (see below) ...
> } else {
> ra_pages = fc->max_read / PAGE_SIZE;
> fc->no_lock = 1;
> @@ -1002,7 +1008,8 @@ void fuse_send_init(struct fuse_conn *fc)
> FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
> FUSE_PARALLEL_DIROPS | FUSE_HANDLE_KILLPRIV | FUSE_POSIX_ACL |
> FUSE_ABORT_ERROR | FUSE_MAX_PAGES | FUSE_CACHE_SYMLINKS |
> - FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA;
> + FUSE_NO_OPENDIR_SUPPORT | FUSE_EXPLICIT_INVAL_DATA |
> + FUSE_PASSTHROUGH;
> ia->args.opcode = FUSE_INIT;
> ia->args.in_numargs = 1;
> ia->args.in_args[0].size = sizeof(ia->in);
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> new file mode 100644
> index 000000000000..86ab4eafa7bf
> --- /dev/null
> +++ b/fs/fuse/passthrough.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "fuse_i.h"
> +
> +int fuse_passthrough_setup(struct fuse_req *req, unsigned int fd)
> +{
> + int ret;
> + int fs_stack_depth;
> + struct file *passthrough_filp;
> + struct inode *passthrough_inode;
> + struct super_block *passthrough_sb;
> +
> + /* Passthrough mode can only be enabled at file open/create time */
> + if (req->in.h.opcode != FUSE_OPEN && req->in.h.opcode != FUSE_CREATE) {
> + pr_err("FUSE: invalid OPCODE for request.\n");
> + return -EINVAL;
> + }
> +
> + passthrough_filp = fget(fd);
> + if (!passthrough_filp) {
> + pr_err("FUSE: invalid file descriptor for passthrough.\n");
> + return -EINVAL;
> + }
> +
> + ret = -EINVAL;
> + if (!passthrough_filp->f_op->read_iter ||
> + !passthrough_filp->f_op->write_iter) {
> + pr_err("FUSE: passthrough file misses file operations.\n");
> + goto out;
> + }
> +
> + passthrough_inode = file_inode(passthrough_filp);
> + passthrough_sb = passthrough_inode->i_sb;
> + fs_stack_depth = passthrough_sb->s_stack_depth + 1;
... for example:
if (fs_stack_depth && passthrough_sb->s_type == fuse_fs_type) {
pr_err("FUSE: stacked passthrough file\n");
goto out;
}
But maybe we want to ban passthrough to any lower FUSE at least for start.
> + ret = -EEXIST;
Why EEXIST? Why not EINVAL?
> + if (fs_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) {
> + pr_err("FUSE: maximum fs stacking depth exceeded for passthrough\n");
> + goto out;
> + }
> +
> + req->args->passthrough_filp = passthrough_filp;
> + return 0;
> +out:
> + fput(passthrough_filp);
> + return ret;
> +}
> +
And speaking of overlayfs, I believe you may be able to test your code with
fuse-overlayfs (passthrough to upper files).
This is a project with real users running real workloads who may be
able to provide you with valuable feedback from testing.
Thanks,
Amir.
[1] https://github.com/containers/fuse-overlayfs
Powered by blists - more mailing lists