[CRIU] [PATCH] mnt: relax constraints on fuse mounts

[Tycho Andersen]

Hi Pavel,

On Thu, Apr 09, 2015 at 08:26:07AM -0600, Tycho Andersen wrote:
> On Thu, Apr 09, 2015 at 12:37:06PM +0300, Pavel Emelyanov wrote:
> > On 04/08/2015 09:13 PM, Tycho Andersen wrote:
> > > 1. If a fuse connection is present, but there are no fuse mounts of that type
> > >    in the mount namespace, don't refuse to dump.
> > > 2. If there are mounts of that type in the container but they are external,
> > >    we're going to bind them anyway, so there's no fuse-specific things that
> > >    need to be done, so it is safe to dump.
> > > 
> > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > > ---
> > >  mount.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 42 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/mount.c b/mount.c
> > > index 71c39bf..700c303 100644
> > > --- a/mount.c
> > > +++ b/mount.c
> > > @@ -1024,6 +1024,47 @@ out:
> > >  	return ret;
> > >  }
> > >  
> > > +static int fusectl_dump(struct mount_info *pm)
> > > +{
> > > +	int fd, ret = -1;
> > > +	struct dirent *de;
> > > +	DIR *fdir = NULL;
> > > +
> > > +	fd = open_mountpoint(pm);
> > > +	if (fd < 0)
> > > +		return -1;
> > > +
> > > +	fdir = fdopendir(fd);
> > > +	if (fdir == NULL) {
> > > +		close(fd);
> > > +		return -1;
> > > +	}
> > > +
> > > +	while ((de = readdir(fdir))) {
> > > +		int id;
> > > +		struct mount_info *it;
> > > +
> > > +		if (dir_dots(de))
> > > +			continue;
> > > +
> > > +		if (sscanf(de->d_name, "%d", &id) != 1) {
> > > +			pr_err("wrong number of items scanned in fusectl dump\n");
> > > +			goto out;
> > > +		}
> > > +
> > > +		for (it = mntinfo; it; it = it->next) {
> > > +			if (id == minor(it->s_dev) && !it->external) {
> > 
> > I'm not sure that minor number match is enough. Should we also check the
> > it->type being fuse?
> 
> I think it should be safe in this case because we're excluding more
> things than we should, rather than allowing stuff through. It is
> better to compare against it->type, though, so I'll resend.

After looking into this a bit more, I don't think we save the fsname
directly, since we resolve it to something in fstypes. The mountinfo
entries for fuse look like:

46 38 0:40 / /run/user/1000/gvfs rw,nosuid,nodev,relatime - fuse.gvfsd-fuse gvfsd-fuse rw,user_id=1000,group_id=1000
49 21 0:43 / /home/tycho/smitten rw,nosuid,nodev,relatime - fuse.sshfs tycho at 192.168.0.69:/home/tycho rw,user_id=1000,group_id=1000

i.e. they have a string like "fuse.foo" for their fstype, which gets resolved
to unsupported. We could either save the raw fstype too, add a special
catch-all type for fuse (with failing dump/restore functions, since we can't
really dump/restore these fs types) or leave the patch as is. Let me know what
changes you want and I can make them.

Tycho

> Tycho
> 
> > > +				pr_err("%s is a fuse mount but not external\n", it->mountpoint);
> > > +				goto out;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	ret = 0;
> > > +out:
> > > +	closedir(fdir);
> > > +	return ret;
> > > +}
> > >  
> > >  static int dump_empty_fs(struct mount_info *pm)
> > >  {
> > > @@ -1102,7 +1143,7 @@ static struct fstype fstypes[] = {
> > >  		.code = FSTYPE__SECURITYFS,
> > >  	}, {
> > >  		.name = "fusectl",
> > > -		.dump = dump_empty_fs,
> > > +		.dump = fusectl_dump,
> > >  		.code = FSTYPE__FUSECTL,
> > >  	}, {
> > >  		.name = "debugfs",
> > > 
> >