[Virtio-fs] [PATCH] virtiofsd: handle NULL dir in lo_do_lookup

[Eryu Guan]

On Tue, May 28, 2019 at 05:29:33PM -0400, Vivek Goyal wrote:
> On Thu, May 23, 2019 at 11:22:54AM +0800, Eryu Guan wrote:
> > On Thu, May 23, 2019 at 10:06:34AM +0800, Liu Bo wrote:
> > > Reported by fstests/generic/467.
> > > 
> > > open_by_handle_at() called from fuse inside guest can carry fuse mount
> > > point to daemon but lo_do_lookup() doesn't know its inode info because
> > > it's out of fuse's scope, thus lo_inode(req, parent) ends up with
> > > returning a NULL dir and breaks virtiofsd immediately.
> > > 
> > > Note that it'd break applications that uses open_by_handle_at.
> > > 
> > > It seems to me that nothing could be done to support open_by_handle_at in
> > > this case.
> > 
> > Then perhaps we should consider disabling exportfs support in virtio-fs
> > case? I noticed that currently fuse set
> > 
> > 	sb->s_export_op = &fuse_export_operations
> > 
> > unconditionally, maybe we could implement a feature mask in fuse and let
> > filesystem implementation registers what features it supports, e.g.
> > exportfs in this case.
> > 
> > Or we could try adding real exportfs support to virtiofs? Though I'm not
> > sure if it's possible or worth it at the moment.
> 
> If we don't support export operations, we should disable it. Looks like
> we have enabled it in virtiofsd.
> 
> lo_init() {
>         if(conn->capable & FUSE_CAP_EXPORT_SUPPORT)
>                 conn->want |= FUSE_CAP_EXPORT_SUPPORT;
> }
> 
> So if we were to disable it, I guess we will have to remove it.

Looks like only removing above lines doesn't disable export support (or
only partially disabled), the test still runs & fails. Removing the
EXPORT_SUPPORT cap only tells fuse to set fc->export_support to 0 and
return -ESTALE on export operations like fuse_get_parent(), but the
name_to_handle_at(2) syscall still could encode file into handle (which
checks sb->s_export_op->fh_to_dentry), so the test thinks virtio-fs
supports exports.

I have to reset sb->s_export_op to NULL when fc->export_support is 0 to
make test not run, as there's no point to have a "half broken" export
operations. This makes me wonder if we could remove
fuse_conn.export_support field all together, as it serves no purpose
when we don't have s_export_op set.

Thanks,
Eryu

> 
> Not sure if it is possible to support export ops in virtio-fs case. Need
> to read more about it.
> 
> Vivek