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

[Arran Cudbard-Bell]

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.

Many Thanks,
-Arran

Arran Cudbard-Bell <a.cudbardb at freeradius.org>
FreeRADIUS Development Team

FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2


#include <talloc.h>
#include <stdio.h>

int main(int argc, char **argv)
{
       TALLOC_CTX *pool = talloc_pool(NULL, 1024);
       TALLOC_CTX *chunk, *fail_chunk;

       if (pool) {
               printf("Pool allocated\n");
       } else {
               printf("Pool allocation failed\n");
       }

       if (talloc_set_memlimit(pool, 512) < 0) {
               printf("talloc_set_memlimit failed\n");
       } else {
              printf("talloc_set_memlimit success\n");
       }

       fail_chunk = talloc_size(pool, 929);
       if (fail_chunk) {
               printf("Chunk allocated (should have failed)\n");
       } else {
               printf("Chunk not allocated (success)\n");
       }
       talloc_free(fail_chunk);

       fail_chunk = talloc_size(pool, 928);
       if (fail_chunk) {
               printf("Chunk allocated (should have failed)\n");
       } else {
               printf("Chunk not allocated (success)\n");
       }
       talloc_free(fail_chunk);

       chunk = talloc_size(pool, 1);
       if (chunk) {
               printf("Chunk allocated\n");
       } else {
               printf("Chunk allocation failed\n");
       }

       chunk = talloc_realloc_size(pool, chunk, 2);
       if (chunk) {
               printf("Chunk realloced\n");
       } else {
               printf("Chunk realloc failed\n");
       }
}