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
trond.myklebust@xxxxxxxxxxxxxxx
