On Thu, Jun 23, 2022 at 03:48:38PM -0500, Seth Forshee wrote:
> On Tue, Jun 21, 2022 at 04:14:54PM +0200, Christian Brauner wrote:
> > diff --git a/fs/attr.c b/fs/attr.c
> > index 88e2ca30d42e..22e310dd483f 100644
> > --- a/fs/attr.c
> > +++ b/fs/attr.c
> > @@ -31,15 +31,15 @@
> > * performed on the raw inode simply passs init_user_ns.
> > */
> > static bool chown_ok(struct user_namespace *mnt_userns,
> > - const struct inode *inode,
> > - kuid_t uid)
> > + const struct inode *inode, vfsuid_t ia_vfsuid)
> > {
> > - kuid_t kuid = i_uid_into_mnt(mnt_userns, inode);
> > - if (uid_eq(current_fsuid(), kuid) && uid_eq(uid, inode->i_uid))
> > + vfsuid_t vfsuid = i_uid_into_vfsuid(mnt_userns, inode);
> > + if (kuid_eq_vfsuid(current_fsuid(), vfsuid) &&
> > + vfsuid_eq(ia_vfsuid, vfsuid))
> > return true;
> > if (capable_wrt_inode_uidgid(mnt_userns, inode, CAP_CHOWN))
> > return true;
> > - if (uid_eq(kuid, INVALID_UID) &&
> > + if (vfsuid_eq(vfsuid, INVALID_VFSUID) &&
>
> If you use my suggestion that comparison to invalid ids should always be
> false, this check will need to change to !vfsuid_valid(vfsuid). I'd
> argue that this makes the code clearer regardless.
>
> > ns_capable(inode->i_sb->s_user_ns, CAP_CHOWN))
> > return true;
> > return false;
> > @@ -58,21 +58,19 @@ static bool chown_ok(struct user_namespace *mnt_userns,
> > * performed on the raw inode simply passs init_user_ns.
> > */
> > static bool chgrp_ok(struct user_namespace *mnt_userns,
> > - const struct inode *inode, kgid_t gid)
> > + const struct inode *inode, vfsgid_t ia_vfsgid)
> > {
> > - kgid_t kgid = i_gid_into_mnt(mnt_userns, inode);
> > - if (uid_eq(current_fsuid(), i_uid_into_mnt(mnt_userns, inode))) {
> > - kgid_t mapped_gid;
> > -
> > - if (gid_eq(gid, inode->i_gid))
> > + vfsgid_t vfsgid = i_gid_into_vfsgid(mnt_userns, inode);
> > + vfsuid_t vfsuid = i_uid_into_vfsuid(mnt_userns, inode);
> > + if (kuid_eq_vfsuid(current_fsuid(), vfsuid)) {
> > + if (vfsgid_eq(ia_vfsgid, vfsgid))
> > return true;
> > - mapped_gid = mapped_kgid_fs(mnt_userns, i_user_ns(inode), gid);
> > - if (in_group_p(mapped_gid))
> > + if (vfsgid_in_group_p(ia_vfsgid))
> > return true;
> > }
> > if (capable_wrt_inode_uidgid(mnt_userns, inode, CAP_CHOWN))
> > return true;
> > - if (gid_eq(kgid, INVALID_GID) &&
> > + if (vfsgid_eq(ia_vfsgid, INVALID_VFSGID) &&
>
> Likewise here.
Agreed.
