On Tue, 26 Apr 2022 at 13:21, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Tue, Apr 26, 2022 at 1:38 PM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > One thing that I just remembered and which I think I haven't mentioned > > so far is that moving S_ISGID stripping from filesystem callpaths into > > the vfs callpaths means that we're hoisting this logic out of vfs_*() > > helpers implicitly. > > > > So filesystems that call vfs_*() helpers directly can't rely on S_ISGID > > stripping being done in vfs_*() helpers anymore unless they pass the > > mode on from a prior run through the vfs. > > > > This mostly affects overlayfs which calls vfs_*() functions directly. So > > a typical overlayfs callstack would be (roughly - I'm omw to lunch): > > > > sys_mknod() > > -> do_mknodat(mode) // calls vfs_prepare_mode() > > -> .mknod = ovl_mknod(mode) > > -> ovl_create(mode) > > -> vfs_mknod(mode) > > > > I think we are safe as overlayfs passes on the mode on from its own run > > through the vfs and then via vfs_*() to the underlying filesystem but it > > is worth point that out. > > > > Ccing Amir just for confirmation. > > Looks fine to me, but CC Miklos ... Looks fine to me as well. Overlayfs should share the mode (including the suid and sgid bits), owner, group and ACL's with the underlying filesystem, so clearing sgid based on overlay parent directory should result in the same mode as if it was done based on the parent directory on the underlying layer. AFAIU this logic is not affected by userns or mnt_userns, but Christian would be best to confirm that. Thanks, Miklos