New talloc feature: memlimits

[Andreas Schneider]

On Saturday 29 September 2012 00:45:20 simo wrote:
> On Sat, 2012-09-29 at 08:27 +1000, Andrew Bartlett wrote:
> > On Sun, 2012-09-23 at 00:10 -0600, idra at samba.org wrote:
> > > Hello list,
> > > 
> > > during the recent SDC Conference we had the Samba4 LDAP server hammered
> > > by
> > > the Codenomicon guys. A few bugs were found where we ended up allocating
> > > huge amounts of memory.
> > > 
> > > These bugs will need fixing, but the situation reminded me that we still
> > > have little or no control on what users can do over ldap. In particular
> > > we have no good way to limit resources, and it is relatively easy to
> > > DoS the LDAP server by making it allocate huge amounts of memory.
> > > 
> > > So I had the idea of limiting memory allocation to arbitrarily settable
> > > sizes based on talloc contextes.
> > > 
> > > Attached you can find an initial implementation of this feature with
> > > basic
> > > tests.
> > > 
> > > By using talloc_set_memlimit() on a context we can decide the maximum
> > > amount of memory that can be used by any alloction on that context or
> > > any of its children. Attempting to allocate more memory than allowed
> > > results in a failed allocation.
> > > Stealing memory under a memlimited hierarchy does not fail even if the
> > > new
> > > total use exceed the limit, but any further allocation on the context
> > > will
> > > fail. This means we'll need to be careful on how we create temporary
> > > contexts and then steal data.
> > > 
> > > Memory limits can nest and any allocation will reflect in the parents
> > > memory limits as well. This allows for a context to have larger limits
> > > and then have individual smaller limits for childrens down the
> > > hierarchy.
> > > 
> > > Well, enough said, if there are any objections on committin gthis change
> > > please speak up, otherwise I will push by the end of the week.
> > 
> > This certainly fits well with the memory model used in the AD DC, where
> > most memory is allocated on a parent context, that eventually ends up at
> > the connecting socket.  It also helps that in most cases we prefer to
> > use this pattern:
> > 
> > TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
> > 
> > (do work)
> > 
> > talloc_steal(mem_ctx, ret)
> > TALLOC_FREE(tmp_ctx)
> > 
> > (rather than steal from a talloc_stackframe() or from a context built on
> > NULL).
> > 
> > It also helps that this isn't a new idea - I remember a discussion with
> > tridge about this early in the new talloc.  (This was in a context of
> > discussions about if we should gracefully handle out of memory at all
> > under a modern unix VM system).
> > 
> > The challenge of course is that finding out what code doesn't deal with
> > memory limits cleanly, and working out what the runtime cost is.  It
> > seems that it would make talloc_steal() a much more intensive operation
> > than it currently is.
> > 
> > Finally, when we do a talloc_steal(), it seems the limit pointer is not
> > updated on the child chunks.  How do we know these limit pointers will
> > remain valid?
> > 
> > I understand your rationale, but I think this needs more work and some
> > very careful positive review rather than a 'push at the end of the
> > week.
> 
> Indeed I went through it with metze and I have a much improved version
> in one of my trees. It passed tests last night but haven't pushed yet.
> 
> You can find it here:
> https://git.samba.org/idra/samba.git/?p=idra/samba.git;a=shortlog;h=refs/hea
> ds/talloc_memlimit

Can you please document the function better. I think it is well explained in 
this thread, but it should also described in more details in the talloc.h 
file. 


	-- andreas

-- 
Andreas Schneider                   GPG-ID: F33E3FC6
Samba Team                             asn at samba.org
www.samba.org