[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140925185311.GA39447@ubuntu-hedt>
Date: Thu, 25 Sep 2014 13:53:11 -0500
From: Seth Forshee <seth.forshee@...onical.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>,
Miklos Szeredi <miklos@...redi.hu>,
"Serge E. Hallyn" <serge@...lyn.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Serge Hallyn <serge.hallyn@...ntu.com>,
fuse-devel <fuse-devel@...ts.sourceforge.net>,
Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux-Fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v2 0/3] fuse: Add support for mounts from pid/user
namespaces
On Thu, Sep 25, 2014 at 01:44:03PM -0500, Seth Forshee wrote:
> On Thu, Sep 25, 2014 at 11:05:36AM -0700, Eric W. Biederman wrote:
> > Miklos Szeredi <miklos@...redi.hu> writes:
> >
> > > On Wed, Sep 24, 2014 at 7:10 PM, Eric W. Biederman
> > > <ebiederm@...ssion.com> wrote:
> > >
> > >
> > >> So in summary I see:
> > >> - Low utility in being able to manipulate files with bad uids.
> > >> - Bad uids are mostly likely malicious action.
> > >> - make_bad_inode is trivial to analyze.
> > >> - No impediments to change if I am wrong.
> > >>
> > >> So unless there is a compelling case, right now I would recommend
> > >> returning -EIO initially. That allows us to concentrate on the easier
> > >> parts of this and it leaves the changes only in fuse.
> > >
> > > The problem with marking the inode bad is that it will mark it bad for
> > > all instances of this filesystem. Including ones which are in a
> > > namespace where the UIDs make perfect sense.
> >
> > There are two cases:
> > app <-> fuse
> > fuse <-> server
> >
> > I proposed mark_bad_inode for "userspace server -> fuse".
> > Where we have one superblock and one server so and one namespace that
> > they decide to talk in when the filesystem was mounted.
> >
> > I think bad_inode is a reasonable response when the filesystem server
> > starts spewing non-sense.
> >
> > > So that really doesn't look like a good solution.
> > >
> > > Doing the check in inode_permission() might be too heavyweight, but
> > > it's still the only one that looks sane.
> >
> > For the "app <-> fuse" case we already have checks in inode_permision
> > that are kuid based that handle that case. We use kuids not for
> > performance (although there is a small advatnage) but to much more to
> > keep the logic simple and maintainable.
> >
> >
> > For the "app -> fuse" case in .setattr we do need a check to verify
> > that the uid and gid are valid. However that check was added with
> > the basic user namespace support and fuse current returns -EOVERFLOW
> > when that happens.
>
> Where does this happen? I haven't managed to track it down yet.
I guess it must be the one in chown_common()? Except that returns
EINVAL, not EOVERFLOW.
>
> I've also added a check in fuse for this. If a uid/gid passed to
> fuse_setattr doesn't map into the namespace it will return -EINVAL.
> Sounds like maybe it should return -EOVERFLOW instead.
>
> Thanks,
> Seth
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists