talloc: talloc_set_memlimit causes all reallocs to fail when used on pools. talloc_set_memlimit not enforced correctly on pools.
jra at samba.org
Fri Oct 16 19:31:36 UTC 2020
On Fri, Oct 16, 2020 at 01:59:11PM -0500, Arran Cudbard-Bell via samba-technical wrote:
> Apologies if this is a repost. The original message didn't make it into the archives, so I'm assuming it also didn't make it to mailing list subscribers.
> First I'd like to thank the contributors to the Samba project. We've been using talloc since ~2013 and it has made an immeasurably large contribution to code clarity and the general stability of FreeRADIUS.
> Reviewing the talloc code shows provisions (and test cases) for applying memlimits to pools, unfortunately in practice, memlimit functionality on pools seems fairly broken.
> 1. Calling talloc_set_memlimit on a pool results in a limit structure being created with limit->cur_size being set to the size of the pool + chunk header + pool header. This seems to effectively mark the entire pool as already in use.
> 2. Because of 1. calling talloc_realloc on a chunk allocated inside a pool always fails (unless the operation would be a noop). Even if the chunk size is well within the pool limits.
> 3. Chunks allocated within a pool are never checked against the memlimit set on the pool. This is actually the only reason why allocations within a pool work at all. If the code path included a call to talloc_memlimit_check, normal allocations would fail the same as calls to talloc_realloc.
> I've tried modifying the code so that only the children of the pool ctx are included in the the cur_size calculation, but this causes a "logic error in talloc_memlimit_shrink" error.
> Happy to keep poking, but I figured someone more familiar with the spirit of the code would probably be able to make more rapid progress.
> I've included some simple test cases at the bottom of this email.
Thanks. Arghhh. I really hate the talloc_memlimit code :-).
Not sure how popular/used it is.
I would love to just strip it out, but as it's part of the
ABI that's too hard. Can you log a bug at bugzilla.samba.org
so we can track this ?
More information about the samba-technical