Seth Forshee <seth.forshee@xxxxxxxxxxxxx> writes: > On Tue, Mar 29, 2016 at 08:36:09PM -0500, Eric W. Biederman wrote: >> Seth Forshee <seth.forshee@xxxxxxxxxxxxx> writes: >> >> > On Fri, Mar 04, 2016 at 04:43:06PM -0600, Eric W. Biederman wrote: >> >> 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 -- 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
