returning values on talloc_tos() (was Re: [PATCH][WIP] Make vfs_acl_xattr use hash of the posix ACL)
jra at samba.org
Tue Oct 9 15:41:04 MDT 2012
On Wed, Oct 10, 2012 at 07:50:03AM +1100, Andrew Bartlett wrote:
> 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.
Theoretically this is true. Pratically it just doesn't happen.
If it did happen we'd have many, many crash bugs already.
ACL modules are rare enough that I'm inclined to just document
this fact and move on.
Remember, when you make a call into a function that doesn't
explicitly have a context yet returns allocated memory, it's
either on malloc, the null context or on talloc_tos(). So
usually you look it up, and by convention in the ACL code
it's been on tos().
Creating new temporary stack frames is actually quite a
rare thing to do and is only done when you know you're
going to be temporarily allocating a lot of memory and
within a call. Normally you just use the tos() that
you got from the inital frame initiating the call from
the client and leave it at that.
Creating a new frame then calling [F]GET_NT_ACL from
within it implies you'll need to copy the ACL off that
frame before freeing the frame.
Right now we only have 3 VFS calls that take an explicit
context. I'm not sure adding context to the ACL calls
is the right move here. It's doable, but I'd like more
input before we decide.
Richard, Volker or Metze, can you comment ? You've
guys have probably written more VFS modules than anyone
Explicit context for [F]GET_NT_ACL() calls or talloc_tos() ?
More information about the samba-technical