[PATCH][WIP] Make vfs_acl_xattr use hash of the posix ACL

[Andrew Bartlett]

On Wed, 2012-10-31 at 07:08 +1100, Andrew Bartlett wrote:
> On Tue, 2012-10-30 at 13:05 -0700, Jeremy Allison wrote:
> > On Tue, Oct 30, 2012 at 02:52:18PM +0100, Christian Ambach wrote:
> > > Hi Andrew,
> > > 
> > > On 10/25/2012 11:52 AM, Andrew Bartlett wrote:
> > > 
> > > >In the meantime, I've finally written the tests I was interested in.
> > > >These now show that the NT ACL invalidation code works correctly, as
> > > >well as testing the mapping of posix ACLs that have never had an NT ACL
> > > >set.
> > > 
> > > I have tried to fit vfs_gpfs to the helper functions and test the result.
> > > 
> > > You can find the patches in my ambi/samba.git repo (fix-acls2 branch).
> > > 
> > > But I was stopped pretty early by spurious smbd aborts and corrupted
> > > ACLs (especially the SIDs) on the wire that even occur when using a
> > > local ext3 + acl_xattr.
> > > 
> > > valgrind reports lots of errors like these:
> > > 
> > > ==25355== Invalid read of size 4
> > > ==25355==    at 0x5C8E83: dom_sid_compare (dom_sid.c:73)
> > > ==25355==    by 0x5C8ED8: dom_sid_equal (dom_sid.c:85)
> > > ==25355==    by 0x5CA7D8: security_token_has_sid (security_token.c:110)
> > > ==25355==    by 0x5ADBCC: se_access_check (access_check.c:229)
> > > ==25355==    by 0x5AE0A2: se_file_access_check (access_check.c:307)
> > > ==25355==    by 0x28643A: smbd_check_access_rights (open.c:137)
> > > ==25355==    by 0x234E6F: dptr_create (dir.c:534)
> > > ==25355==    by 0x276CB0: call_trans2findfirst (trans2.c:2499)
> > > ==25355==    by 0x27BA20: handle_trans2 (trans2.c:8583)
> > > ==25355==    by 0x27D5EF: reply_trans2 (trans2.c:8869)
> > > ==25355==    by 0x2A7ED2: switch_message (process.c:1551)
> > > ==25355==    by 0x2A995C: process_smb (process.c:1587)
> > > ==25355==  Address 0xf3e197c is 156 bytes inside a block of size 792 free'd
> > > ==25355==    at 0x4C23D72: free (vg_replace_malloc.c:325)
> > > ==25355==    by 0x84D5ED: _talloc_free_internal (talloc.c:942)
> > > ==25355==    by 0x84B522: _talloc_free (talloc.c:1355)
> > > ==25355==    by 0x108AFE94: get_nt_acl_internal (vfs_acl_common.c:673)
> > > ==25355==    by 0x108B0EC5: get_nt_acl_common (vfs_acl_common.c:701)
> > > ==25355==    by 0x2918FC: smb_vfs_call_get_nt_acl (vfs.c:2183)
> > > ==25355==    by 0x28640C: smbd_check_access_rights (open.c:115)
> > > ==25355==    by 0x234E6F: dptr_create (dir.c:534)
> > > ==25355==    by 0x276CB0: call_trans2findfirst (trans2.c:2499)
> > > ==25355==    by 0x27BA20: handle_trans2 (trans2.c:8583)
> > > ==25355==    by 0x27D5EF: reply_trans2 (trans2.c:8869)
> > > ==25355==    by 0x2A7ED2: switch_message (process.c:1551)
> > > 
> > > 
> > > I suspect that your introduction of a stackframe in get_nt_acl_common()
> > > now reveals that the POSIX ACL code has a weird memory hierarchy and
> > > also relies on talloc_tos() pointing to the same stackframe as for the
> > > upper callers in smbd main code.
> > 
> > Ok, I've tried in master and can't reproduce this.
> > 
> > Can you let me know how you're getting this problem to occur ?
> 
> I'm onto this one.  When I converted the get_nt_acl VFS call to take a
> mem_ctx, I didn't catch all the talloc_tos() users.  I'm going over the
> posix_acls.c file to pass down the mem_ctx to the right places.

So much for that theory, the remaining talloc_tos() calls are all
reasonable or at least only on the 'set' side.  Clearly at some layer we
don't honour the interface promise and allocate the returned NT ACL
fully on mem_ctx.  The placement of the talloc_stackframe() simply means
we actually notice the incorrect memory tree.

However, it's not in the posix_acls or vfs_acl_common code as far as I
can tell, because valgrind over that code (now quite well exercised by
samba.tests.posixacl) is clean.

After setting up things by running enough of make test to run any
plugin_s4_dc test, I ran:

SELFTEST_PREFIX=st SMB_CONF_PATH=st/plugin_s4_dc/etc/smb.conf
PYTHONPATH=bin/python:/data/samba/git/samba/bin/python:/data/samba/git/samba/lib/subunit/python:/data/samba/git/samba/lib/testtools valgrind python -m subunit.run $LISTOPT samba.tests.posixacl

Christian,

Given this, can you try and reproduce with and without gpfs, with and
without vfs_acl_xattr, and with and without NFSv4 ACLs on GPFS, to try
and help us narrow this down?

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org