Re: [PATCH v2 2/2] nfs: NFSv3: fix SGID bit dropped when inheriting ACLs

From: Trond Myklebust
Date: Thu Jun 24 2021 - 09:13:56 EST

On Wed, 2021-06-23 at 14:38 +0800, Gao Xiang wrote:
> generic/444 fails with NFSv3 as shown above, "
>      QA output created by 444
>      drwxrwsr-x
>     -drwxrwsr-x
>     +drwxrwxr-x
> ", which tests "SGID inheritance with default ACLs" fs regression
> and looks after the following commits:
> a3bb2d558752 ("ext4: Don't clear SGID when inheriting ACLs")
> 073931017b49 ("posix_acl: Clear SGID bit when setting file
> permissions")
> commit 055ffbea0596 ("[PATCH] NFS: Fix handling of the umask when
> an NFSv3 default acl is present.") sets acls explicitly when
> when files are created in a directory that has a default ACL.
> However, after commit a3bb2d558752 and 073931017b49, SGID can be
> dropped if user is not member of the owning group with
> set_posix_acl() called.
> Since underlayfs will handle ACL inheritance when creating
> files in a directory that has the default ACL and the umask is
> supposed to be ignored for such case. Therefore, I think no need
> to set acls explicitly (to avoid SGID bit cleared) but only apply
> client umask if the default ACL of the parent directory doesn't
> exist.

Hmm... Has this patch been tested with a Solaris server? Your assertion
above appears to be true for Linux servers, but this code needs to
interoperate with non-Linux draft posix acl compatible servers too.

I've already taken the other patch in this series, since that one
appears correct, and doesn't have any interoperability consequences.

Trond Myklebust
Linux NFS client maintainer, Hammerspace