Messages in this thread | | | Date | Thu, 16 Jun 2022 09:59:15 +0200 | From | Christian Brauner <> | Subject | Re: [GIT PULL] fs fix for v5.19-rc3 |
| |
On Wed, Jun 15, 2022 at 09:31:32AM -0700, Linus Torvalds wrote: > On Wed, Jun 15, 2022 at 3:39 AM Christian Brauner <brauner@kernel.org> 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.
I'm actually looking into this and have been for a while. I had thought about this from the start but just now have time to play with this!
| |