Re: [PATCH RESEND v2 11/18] fs: Ensure the mounter of a filesystem is privileged towards its inodes

Seth Forshee [off-list ref] writes:

On Tue, Mar 29, 2016 at 08:36:09PM -0500, Eric W. Biederman wrote:

quoted

Seth Forshee [off-list ref] writes:

quoted

On Fri, Mar 04, 2016 at 04:43:06PM -0600, Eric W. Biederman wrote:

quoted

In general this is only an issue if uids and gids on the filesystem
do not map into the user namespace.

Therefore the general fix is to limit the logic of checking for
capabilities in s_user_ns if we are dealing with INVALID_UID and
INVALID_GID.  For proc and kernfs that should never be the case
so the problem becomes a non-issue.

Further I would look at limiting that relaxation to just
inode_change_ok. 

Finally got around to implementing this today; is the patch below what
you had in mind?

Pretty much.

For the same reason that capble_wrt_inode_uidgid(inode) had to look
at both inode->i_uid and inode->i_gid I think we need to look at
both inode->i_uid and inode->i_gid in those case.

I am worried about chgrp_ok in cases such as inode->i_uid is valid
but unmapped.  I have a similiar worry about chown_ok where
inode->i_gid is valid but unmapped (although that worry is less
serious).

That makes sense.

So then what is wanted is to check that the other id is either invalid,
or else it maps into s_user_ns. So for chown_ok() something like this:

    if (!uid_valid(inode->i_uid) &&
        (!gid_valid(inode->i_gid) || kgid_has_mapping(inode->i_sb->s_user_ns, inode->i_gid)) &&
        ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
            return true;

and likewise for chgrp_ok(). Does that satisfy your concerns?

Yes it does.

Eric