Re: [PATCH V10 2/5] fuse: Passthrough initialization and release

[Alessio Balsini <balsini@android.com>]

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