Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 27 Nov 2020 13:41:23 +0000
From:   Alessio Balsini <balsini@android.com>
To:     Peng Tao <bergwolf@gmail.com>
Cc:     Alessio Balsini <balsini@android.com>,
        Miklos Szeredi <miklos@szeredi.hu>,
        Akilesh Kailash <akailash@google.com>,
        Amir Goldstein <amir73il@gmail.com>,
        Antonio SJ Musumeci <trapexit@spawn.link>,
        David Anderson <dvander@google.com>,
        Giuseppe Scrivano <gscrivan@redhat.com>,
        Jann Horn <jannh@google.com>, Jens Axboe <axboe@kernel.dk>,
        Martijn Coenen <maco@android.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Lawrence <paullawrence@google.com>,
        Stefano Duo <duostefano93@gmail.com>,
        Zimuzo Ezeozue <zezeozue@google.com>,
        fuse-devel@lists.sourceforge.net, kernel-team@android.com,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V10 2/5] fuse: Passthrough initialization and release
Message-ID: <20201127134123.GA569154@google.com>
References: <20201026125016.1905945-1-balsini@android.com>
 <20201026125016.1905945-3-balsini@android.com>
 <CA+a=Yy4bhC-432h8shxbsrY5vjTcRZopS-Ojo0924L49+Be3Cg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+a=Yy4bhC-432h8shxbsrY5vjTcRZopS-Ojo0924L49+Be3Cg@mail.gmail.com>
Precedence: bulk

Hi Peng,

Thanks for the heads up!

On Thu, Nov 26, 2020 at 09:33:34PM +0800, Peng Tao wrote:
> On Tue, Oct 27, 2020 at 12:19 AM Alessio Balsini <balsini@android.com> wrote:
> > [...]
> >  int fuse_passthrough_setup(struct fuse_conn *fc, struct fuse_file *ff,
> >                            struct fuse_open_out *openarg)
> >  {
> > -       return -EINVAL;
> > +       struct inode *passthrough_inode;
> > +       struct super_block *passthrough_sb;
> > +       struct fuse_passthrough *passthrough;
> > +       int passthrough_fh = openarg->passthrough_fh;
> > +
> > +       if (!fc->passthrough)
> > +               return -EPERM;
> > +
> > +       /* Default case, passthrough is not requested */
> > +       if (passthrough_fh <= 0)
> > +               return -EINVAL;
> > +
> > +       spin_lock(&fc->passthrough_req_lock);
> > +       passthrough = idr_remove(&fc->passthrough_req, passthrough_fh);
> > +       spin_unlock(&fc->passthrough_req_lock);
> > +
> > +       if (!passthrough)
> > +               return -EINVAL;
> > +
> > +       passthrough_inode = file_inode(passthrough->filp);
> > +       passthrough_sb = passthrough_inode->i_sb;
> > +       if (passthrough_sb->s_stack_depth >= FILESYSTEM_MAX_STACK_DEPTH) {
> Hi Alessio,
> 
> passthrough_sb is the underlying filesystem superblock, right? It
> seems to prevent fuse passthrough fs from stacking on another fully
> stacked file system, instead of preventing other file systems from
> stacking on this fuse passthrough file system. Am I misunderstanding
> it?

Correct, this checks the stacking depth on the lower filesystem.
This is an intended behavior to avoid the stacking of multiple FUSE
passthrough filesystems, and works because when a FUSE connection has
the passthrough feature activated, the file system updates its
s_stack_depth to FILESYSTEM_MAX_STACK_DEPTH in process_init_reply()
(PATCH 1/5), avoiding further stacking.

Do you see issues with that?

What I'm now thinking is that fuse_passthrough_open would probably be a
better place for this check, so that the ioctl() would fail and the user
space daemon may decide to skip passthrough and use legacy FUSE access
for that file (or, at least, be aware that something went wrong).

A more aggressive approach would be instead to move the stacking depth
check to fuse_fill_super_common, where we can update s_stack_depth to
lower-fs depth+1 and fail if passthrough is active and s_stack_depth is
greater than FILESYSTEM_MAX_STACK_DEPTH.

What do you think?

Thanks,
Alessio