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 19, 2020, at 10:53 PM, Jeremy Allison <jra at samba.org> wrote:
> 
> On Mon, Oct 19, 2020 at 07:02:46PM -0700, Jeremy Allison via samba-technical wrote:
>> On Fri, Oct 16, 2020 at 07:52:48PM -0500, Arran Cudbard-Bell via samba-technical wrote:
>>> 
>>>>> An alternative that'd still satisfy our immediate need would be to have talloc_set_memlimit simply fail when someone tried to apply it to a pool (as you suggested), and add an optional flag that'd prevent allocations from occurring outside of the pool.
>>>> 
>>>> Please wrap your responses to 80 columns :-). Makes
>>>> quoting your replies really hard :-).
>>> 
>>> Will do :)
>>> 
>>>> 
>>>> What you're asking for is more complexity in an
>>>> already overly complex part of the code (which
>>>> to be honest I wasn't even sure people were
>>>> using :-).
>>>> 
>>>> I think you can do what you need by allocating
>>>> a pool as a talloc child of a context, and setting
>>>> the memlimit on the that context.
>>> 
>>> I just tried this and it didn't work, the reallocs still fail.
>>> 
>>> This is likely because the limit needs to be the size of the pool plus 
>>> headers.  I don't believe there's any way for the caller to know the size
>>> of these headers, but maybe you know better :)
>>> 
>>> talloc_get_size() returns 0 when called on the ctx or the pool as the
>>> docs suggest it should.
>>> 
>>> Do you have any idea how I could determine the correct value to
>>> pass to talloc_set_memlimit?
>>> 
>>> -Arran
>> 
>> I think it's a bug. If you can rebuild talloc can you
>> check with this (not well tested yet :-) patch ?
>> 
>> I'm planning to add regression tests around this.
>> 
>> With this patch talloc_memlimits on pools should
>> work.
> 
> Slightly better version that does the size accounting
> correctly in the 'can't allocate from existing pool'
> case.

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.

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.

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.

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?

Many Thanks,
-Arran