returning values on talloc_tos() (was Re: [PATCH][WIP] Make vfs_acl_xattr use hash of the posix ACL)

[Andrew Bartlett]

On Tue, 2012-10-09 at 12:12 -0700, Jeremy Allison wrote:
> On Tue, Oct 09, 2012 at 10:58:17PM +1100, Andrew Bartlett wrote:
> > This patch (still a work in progress, as I know it fails some tests) is
> > what I've been talking about for months, and which I got VFS changes in
> > (almost - one small change in this patch) into 4.0.0rc1.
> > 
> > The idea is simple, have the ACL modules (the posix ACL modules in
> > particular) provide a blob form of the ACL, and then hash that, rather
> > than an NT ACL that might change if our ACL mapping code changes. 
> > 
> > So far I've made it simply fall back in the NFSv4 case, but we can
> > define these in IDL as well if we want to do the same there. 
> > 
> > The idea is to insulate these on-disk values from any required change we
> > have to make. 
> > 
> > I'm proposing these changes (once finished) for master, and will son
> > have some more tests for this code.
> > 
> > Any thoughts or comments most appreciated.
> 
> I'm not very keen on the talloc_steal code in here.
> 
> Why are you moving the talloc hierarcies around ?
> 
> The current ACL code is very simple, in that the
> returned ACLs to any FGET_NT_ACL/GET_NT_ACL are
> always allocated on the talloc_tos() of the caller.
> 
> There are no cases I know of where the context is
> anything other than containing talloc_tos() (actually
> this causes an issue in one place, but the place to
> fix that is in the caller of [F]GET_NT_ACL).

We really need to fix that, which is why I never suspected it.  

We should never return a value allocated on talloc_tos(), because any
layer in between might call talloc_stackframe()/talloc_free(), totally
invalidating the retuned data.

That layer could be a VFS module, which is quite likely given they can
be stacked. 

I'll work on patches to provide a caller-supplied context shortly.
While I'm at it, are you aware of any other VFS calls that would need to
be converted?  Rusty worked on much of the rest of the code a few months
back, passing in a memory context to lp_() functions that use
talloc_tos() for example, so I'm actually supprised this was missed in
his tests.  (I think he (ab)used the gcov hooks to put a
talloc_stackframe() in every function).  

Andrew Bartlett

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