Re: [GIT PULL] fs fix for v5.19-rc3
From: Linus Torvalds
Date: Wed Jun 15 2022 - 12:31:56 EST
On Wed, Jun 15, 2022 at 3:39 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> This fixes an issue where we fail to change the group of a file when the caller
> owns the file and is a member of the group to change to. This is only relevant
> on idmapped mounts.
Pulled, but this code makes me go "Eww".
I do get the feeling that maybe we should have some kind of static
type help here, with "unmapped" and "filesystem mapped" uid/gid values
having different types, the same way we have kuid_t vs uid_t for
user-namespace mapping.
Because it feels like the problem here is that "kgid_t" has different
meanings depending on context.
In fact, just within that one function, we have *three* very different kgid_t's:
- the argument to the function is a "kgid_t gid", which has *not*
been mapped into the filesystem mapping (why not?)
- the function itself declares a "kgid_t kgid", which is the inode
group ID mapped into the mount
- it how additionally has that thied "kgid_t mapped_gid", which is
the first 'gid' mapped into the mount
and honestly, I find that hugely confusing. The naming doesn't exactly
clarify things either (ie "gid" is a "kgid_t", not a "git_t", and so
is "kgid" - not helpful).
And being confusing with no type system support isn't a great thing.
So I'm wondering if <linux/mnt_idmapping.h> might be able to do the
same kind of things we do in <linux/uidgid.h>.
NOTE: I did not look into just how hugely painful that would be. Maybe
the above is the rantings of a madman.
Linus