Re: [PATCH v8 3/4] fs: move S_ISGID stripping into the vfs

[Miklos Szeredi <miklos@szeredi.hu>]

On Tue, 26 Apr 2022 at 13:21, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Apr 26, 2022 at 1:38 PM Christian Brauner <brauner@kernel.org> 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