talloc: talloc_set_memlimit causes all reallocs to fail when used on pools. talloc_set_memlimit not enforced correctly on pools.

[Arran Cudbard-Bell]

> On Oct 20, 2020, at 12:50 AM, Jeremy Allison <jra at samba.org> wrote:
> 
> On Mon, Oct 19, 2020 at 11:17:29PM -0500, Arran Cudbard-Bell via samba-technical wrote:
>> Much appreciated!  I'll try this out tomorrow.
>> 
>> Just so I'm clear on the expected behaviour, could
>> you verify that these assumptions are correct:
>> 
>> 1. If there's a 1024 byte pool, with a 2048 byte memlimit,
>> the amount of memory allocated outside of the pool would 
>> be limited to 1024 bytes.
> 
> Yes. Pool == 1024 bytes allocated, any extra must be
> below total size (including pool, which is treated
> as allocated).
> 
>> 2. If there's a 1024 byte pool, with a 1024 byte memlimit,
>> any alloc or realloc up to the pool size would succeed,
>> but any alloc or realloc which'd result in a chunk being
>> allocated outside of pool memory would fail.
> 
> Yes. memlimit == pool size, so any further mallocs/reallocs
> adding memory should fail.
> 
>> 3. If there's a 1024 byte pool, with a 512 byte memlimit,
>> the memlimit would cause all reallocs and allocs to
>> fail, because the pool size is already greater than the
>> memlimit.
> 
> No. Allocations inside the pool below 1024 bytes would
> always succeed (until the pool is filled) as when allocating
> from a pool the memlimit isn't checked if the allocation
> can be filled from within the pool.
> 
> memlimit is only checked in actual malloc/realloc calls.
> 
> If a pool already exists, it's assumed that it must already
> have fit within the memlimit. It makes no sense to
> set the limit lower than the pool size.
> 
>> Additionally, if the third assumption is correct, do you 
>> see any value in instead having talloc_set_memlimit 
>> fail if a caller specifies a memlimit smaller than the pool?
> 
> Nope. See above.

OK.  That patch seems to mostly work but there are a few
issues.

First is an easy one. After a realloc failure due to hitting the
memlimit, the chunk remains marked as free, so when a
report gets run later in the test suite, we get a use-after-free
error.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Mark-chunks-that-failed-realloc-as-not-free.patch
Type: application/octet-stream
Size: 1545 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20201020/d42e0e90/0001-Mark-chunks-that-failed-realloc-as-not-free.obj>
-------------- next part --------------


Then there's another issue with object_count ending
up off by one, which means talloc_free_children doesn't
actually return memory to the pool, and that messes up
some of the other tests I'm adding.  Just tracking down
when and why this happens now.... It might have been a
pre-existing issue and not related to this patch, I'm just
seeing it because of using talloc_free_children to reset
the pool between some tests.

-Arran