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

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@hammerspace.com