Re: [PATCH v2 8/8] attr: port attribute changes to new types

[Seth Forshee <sforshee@digitalocean.com>]

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.