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

[Andrew Bartlett]

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.

Andrew Bartlett

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