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@xxxxxxxxxx> writes: > > > > > On Wed, Sep 24, 2014 at 7:10 PM, Eric W. Biederman > > > <ebiederm@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx > 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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
