[PATCH] VFS cleanup - remove chmod_acl and fchmod_acl

[Jeremy Allison]

On Thu, May 17, 2018 at 05:17:30PM -0700, Jeremy Allison wrote:
> On Thu, May 17, 2018 at 02:37:15PM -0700, Jeremy Allison wrote:
> > Hi all,
> > 
> > After a conversation with Volker, I realized it's
> > time (for 4.9.0) to remove the hideous VFS
> > calls SMB_VFS_CHMOD_ACL() and SMB_VFS_FCHMOD_ACL().
> > 
> > They exist because I utterly misunderstood the
> > meaning of the ACL_MASK ace entry in POSIX
> > ACLs when I was first writing an ACL backend
> > for Samba.
> > 
> > I assumed that for ACL mapping from Windows
> > we would always have to set the ACL_MASK to
> > "rwx", so hand-implemented the copying of
> > u/g/o bits on a mode change to individual
> > ACE entries in a POSIX ACL (see the code
> > in chmod_acl_internals() in source3/smbd/posix_acls.c
> > for details).
> > 
> > This is just silly - doing a [f]chmod
> > implements exactly the same effect
> > by changing the mask bits (and is
> > correctly read and applied in canonicalise_acl(),
> > also in source3/smbd/posix_acls.c).
> > 
> > So this patchset removes the two
> > VFS calls SMB_VFS_CHMOD_ACL() and SMB_VFS_FCHMOD_ACL()
> > and replaces their use with simple SMB_VFS_CHMOD()
> > and SMB_VFS_FCHMOD() instead.
> > 
> > This change should only be visible to
> > unix extensions POSIX clients doing
> > direct chmod and posix ACL get/set calls,
> > and it will cause Samba to be closer
> > to expected POSIX behavior.
> 
> Ha, don't push yet ! It seems to have
> found a bug in the torture_samba3_hide()
> test (which does do direct POSIX chmod
> calls :-). I think that bug was hidden
> before this patchset (and that's why
> the samba3checkfsp test was being
> marked flakey, a dead file is being
> left behind that should be there... :-).
> 
> More when I've figured out the problem
> with the test :-).

Figured it out (vfs_fake_acl also needs
to implement chown()/fchown()).

The nice thing is now I've done that I'm
able to remove a 'knownfail' entry as
the POSIX ACL tests start working on
the nt_dc backend (which uses vfs_fake_acl).

There's also a bug with samba3hide failing
and leaving dead files around with the same
name as files used by samba3checkfsp, but
that's a different issue (and I've logged
a new bug for that). I think I can clean
up some of these older tests to fail-safe
and remove test interactions, which should
help make our autobuild process more robust,
- but that's a patch for another day :-).

I'll re-post a new complete patch once
I'm convinced it's passing all make tests.

Jeremy