The Samba-Bugzilla – Attachment 16313 Details for
Bug 14540
talloc_set_memlimit causes all reallocs to fail when used on pools.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for master.
bug-14540-master (text/plain), 13.13 KB, created by
Jeremy Allison
on 2020-10-27 21:40:13 UTC
(
hide
)
Description:
git-am fix for master.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2020-10-27 21:40:13 UTC
Size:
13.13 KB
patch
obsolete
>From 02a7dba9acff2bb359c322217d0c8a8bb38105ab Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 20 Oct 2020 12:18:10 -0700 >Subject: [PATCH 1/5] lib: talloc: Cleanup. Use consistent preprocessor logic > macros. > >Match other use of ALWAYS_REALLOC. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14540 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > lib/talloc/talloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index e476f3e2d05..54250c1b67d 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1898,7 +1898,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > */ > _talloc_chunk_set_free(tc, NULL); > >-#if ALWAYS_REALLOC >+#if (ALWAYS_REALLOC != 0) > if (pool_hdr) { > new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0); > pool_hdr->object_count--; >-- >2.25.1 > > >From beeacae9c1b666a1eea31fb76ca08e0ccdc7d254 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 20 Oct 2020 12:14:58 -0700 >Subject: [PATCH 2/5] lib: talloc: Fix pool object accounting when doing > talloc_realloc() in the ALWAYS_REALLOC compiled case. > >tc_alloc_pool() or the fallback malloc can return NULL. > >Wait until we know we are returning a valid pointer >before decrementing pool_hdr->object_count due to >reallocing out of the talloc_pool. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14540 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > lib/talloc/talloc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 54250c1b67d..885705234d4 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1901,8 +1901,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > #if (ALWAYS_REALLOC != 0) > if (pool_hdr) { > new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0); >- pool_hdr->object_count--; >- > if (new_ptr == NULL) { > new_ptr = malloc(TC_HDR_SIZE+size); > malloced = true; >@@ -1912,6 +1910,11 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > if (new_ptr) { > memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE); > TC_INVALIDATE_FULL_CHUNK(tc); >+ /* >+ * Only decrement the object count in the pool once >+ * we know we're returning a valid new_ptr. >+ */ >+ pool_hdr->object_count--; > } > } else { > /* We're doing malloc then free here, so record the difference. */ >-- >2.25.1 > > >From a92a9a1cd25d93237dd8617efdfe6c89e672f361 Mon Sep 17 00:00:00 2001 >From: Arran Cudbard-Bell <a.cudbardb@freeradius.org> >Date: Tue, 20 Oct 2020 14:10:30 -0500 >Subject: [PATCH 3/5] lib: talloc: Add more debugging text for existing > memlimit + pool tests > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14540 > >Signed-off-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >--- > lib/talloc/testsuite.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > >diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c >index 6c9bf203b7c..571c32ca00f 100644 >--- a/lib/talloc/testsuite.c >+++ b/lib/talloc/testsuite.c >@@ -1767,24 +1767,38 @@ static bool test_memlimit(void) > talloc_free(root); > > /* Test memlimits with pools. */ >+ printf("==== talloc_pool(NULL, 10*1024)\n"); > pool = talloc_pool(NULL, 10*1024); > torture_assert("memlimit", pool != NULL, > "failed: alloc should not fail due to memory limit\n"); >+ >+ printf("==== talloc_set_memlimit(pool, 10*1024)\n"); > talloc_set_memlimit(pool, 10*1024); > for (i = 0; i < 9; i++) { >+ printf("==== talloc_size(pool, 1024) %i/10\n", i + 1); > l1 = talloc_size(pool, 1024); > torture_assert("memlimit", l1 != NULL, > "failed: alloc should not fail due to memory limit\n"); >+ talloc_report_full(pool, stdout); > } > /* The next alloc should fail. */ >+ printf("==== talloc_size(pool, 1024) 10/10\n"); > l2 = talloc_size(pool, 1024); > torture_assert("memlimit", l2 == NULL, > "failed: alloc should fail due to memory limit\n"); > >+ talloc_report_full(pool, stdout); >+ > /* Moving one of the children shouldn't change the limit, > as it's still inside the pool. */ >+ >+ printf("==== talloc_new(NULL)\n"); > root = talloc_new(NULL); >+ >+ printf("==== talloc_steal(root, l1)\n"); > talloc_steal(root, l1); >+ >+ printf("==== talloc_size(pool, 1024)\n"); > l2 = talloc_size(pool, 1024); > torture_assert("memlimit", l2 == NULL, > "failed: alloc should fail due to memory limit\n"); >-- >2.25.1 > > >From 7b3b9c4524dbeedc28949c7c46b6e64c43abe845 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 20 Oct 2020 10:52:55 -0700 >Subject: [PATCH 4/5] lib: talloc: Fix memlimit on pool realloc. > >We only have to do the memlimit check before any >real malloc or realloc. Allocations out of a >memory pool have already been counted in the >memory limit, so don't check in those cases. > >This is an application-visible change (although >fixing a bug) so bump the ABI to 2.3.1 -> 2.3.2. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14540 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Signed-off-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org> >--- > lib/talloc/talloc.c | 69 ++++++++++++++++++++++++++++++++++++++++----- > lib/talloc/wscript | 2 +- > 2 files changed, 63 insertions(+), 8 deletions(-) > >diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c >index 885705234d4..078e58ca352 100644 >--- a/lib/talloc/talloc.c >+++ b/lib/talloc/talloc.c >@@ -1833,13 +1833,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > return NULL; > } > >- if (tc->limit && (size > tc->size)) { >- if (!talloc_memlimit_check(tc->limit, (size - tc->size))) { >- errno = ENOMEM; >- return NULL; >- } >- } >- > /* handle realloc inside a talloc_pool */ > if (unlikely(tc->flags & TALLOC_FLAG_POOLMEM)) { > pool_hdr = tc->pool; >@@ -1902,6 +1895,25 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > if (pool_hdr) { > new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0); > if (new_ptr == NULL) { >+ /* >+ * Couldn't allocate from pool (pool size >+ * counts as already allocated for memlimit >+ * purposes). We must check memory limit >+ * before any real malloc. >+ */ >+ if (tc->limit) { >+ /* >+ * Note we're doing an extra malloc, >+ * on top of the pool size, so account >+ * for size only, not the difference >+ * between old and new size. >+ */ >+ if (!talloc_memlimit_check(tc->limit, size)) { >+ _talloc_chunk_set_not_free(tc); >+ errno = ENOMEM; >+ return NULL; >+ } >+ } > new_ptr = malloc(TC_HDR_SIZE+size); > malloced = true; > new_size = size; >@@ -1920,6 +1932,18 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > /* We're doing malloc then free here, so record the difference. */ > old_size = tc->size; > new_size = size; >+ /* >+ * We must check memory limit >+ * before any real malloc. >+ */ >+ if (tc->limit && (size > old_size)) { >+ if (!talloc_memlimit_check(tc->limit, >+ (size - old_size))) { >+ _talloc_chunk_set_not_free(tc); >+ errno = ENOMEM; >+ return NULL; >+ } >+ } > new_ptr = malloc(size + TC_HDR_SIZE); > if (new_ptr) { > memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE); >@@ -2023,6 +2047,25 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0); > > if (new_ptr == NULL) { >+ /* >+ * Couldn't allocate from pool (pool size >+ * counts as already allocated for memlimit >+ * purposes). We must check memory limit >+ * before any real malloc. >+ */ >+ if (tc->limit) { >+ /* >+ * Note we're doing an extra malloc, >+ * on top of the pool size, so account >+ * for size only, not the difference >+ * between old and new size. >+ */ >+ if (!talloc_memlimit_check(tc->limit, size)) { >+ _talloc_chunk_set_not_free(tc); >+ errno = ENOMEM; >+ return NULL; >+ } >+ } > new_ptr = malloc(TC_HDR_SIZE+size); > malloced = true; > new_size = size; >@@ -2038,6 +2081,18 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons > /* We're doing realloc here, so record the difference. */ > old_size = tc->size; > new_size = size; >+ /* >+ * We must check memory limit >+ * before any real realloc. >+ */ >+ if (tc->limit && (size > old_size)) { >+ if (!talloc_memlimit_check(tc->limit, >+ (size - old_size))) { >+ _talloc_chunk_set_not_free(tc); >+ errno = ENOMEM; >+ return NULL; >+ } >+ } > new_ptr = realloc(tc, size + TC_HDR_SIZE); > } > got_new_ptr: >diff --git a/lib/talloc/wscript b/lib/talloc/wscript >index b955d215b80..a767477357f 100644 >--- a/lib/talloc/wscript >+++ b/lib/talloc/wscript >@@ -1,7 +1,7 @@ > #!/usr/bin/env python > > APPNAME = 'talloc' >-VERSION = '2.3.1' >+VERSION = '2.3.2' > > import os > import sys >-- >2.25.1 > > >From 15e002274b88df4c53c767d1ddcf3d808e1a8208 Mon Sep 17 00:00:00 2001 >From: Arran Cudbard-Bell <a.cudbardb@freeradius.org> >Date: Tue, 20 Oct 2020 14:12:17 -0500 >Subject: [PATCH 5/5] lib: talloc: More tests for realloc when used with > memlimited pools > >This requires the previous patch. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14540 > >Signed-off-by: Arran Cudbard-Bell <a.cudbardb@freeradius.org> >Reviewed-by: Jeremy Allison <jra@samba.org> >--- > lib/talloc/testsuite.c | 103 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 103 insertions(+) > >diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c >index 571c32ca00f..6f23ad4e18a 100644 >--- a/lib/talloc/testsuite.c >+++ b/lib/talloc/testsuite.c >@@ -1803,6 +1803,109 @@ static bool test_memlimit(void) > torture_assert("memlimit", l2 == NULL, > "failed: alloc should fail due to memory limit\n"); > >+ printf("==== talloc_free_children(pool)\n"); >+ talloc_free(l1); >+ talloc_free_children(pool); >+ >+ printf("==== talloc_size(pool, 1024)\n"); >+ l1 = talloc_size(pool, 1024); >+ >+ /* try reallocs of increasing size */ >+ for (i = 1; i < 9; i++) { >+ printf("==== talloc_realloc_size(NULL, l1, %i*1024) %i/10\n", i, i + 1); >+ l1 = talloc_realloc_size(NULL, l1, i*1024); >+ torture_assert("memlimit", l1 != NULL, >+ "failed: realloc should not fail due to memory limit\n"); >+ talloc_report_full(pool, stdout); >+ } >+ /* The next alloc should fail. */ >+ printf("==== talloc_realloc_size(NULL, l1, 10*1024) 10/10\n"); >+ l2 = talloc_realloc_size(NULL, l1, 10*1024); >+ torture_assert("memlimit", l2 == NULL, >+ "failed: realloc should fail due to memory limit\n"); >+ >+ /* Increase the memlimit */ >+ printf("==== talloc_set_memlimit(pool, 11*1024)\n"); >+ talloc_set_memlimit(pool, 11*1024); >+ >+ /* The final realloc should still fail >+ as the entire realloced chunk needs to be moved out of the pool */ >+ printf("==== talloc_realloc_size(NULL, l1, 10*1024) 10/10\n"); >+ l2 = talloc_realloc_size(NULL, l1, 10*1024); >+ torture_assert("memlimit", l2 == NULL, >+ "failed: realloc should fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ printf("==== talloc_set_memlimit(pool, 21*1024)\n"); >+ talloc_set_memlimit(pool, 21*1024); >+ >+ /* There's now sufficient space to move the chunk out of the pool */ >+ printf("==== talloc_realloc_size(NULL, l1, 10*1024) 10/10\n"); >+ l2 = talloc_realloc_size(NULL, l1, 10*1024); >+ torture_assert("memlimit", l2 != NULL, >+ "failed: realloc should not fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ /* ...which should mean smaller allocations can now occur within the pool */ >+ printf("==== talloc_size(pool, 9*1024)\n"); >+ l1 = talloc_size(pool, 9*1024); >+ torture_assert("memlimit", l1 != NULL, >+ "failed: new allocations should be allowed in the pool\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ /* But reallocs bigger than the pool will still fail */ >+ printf("==== talloc_realloc_size(NULL, l1, 10*1024)\n"); >+ l2 = talloc_realloc_size(NULL, l1, 10*1024); >+ torture_assert("memlimit", l2 == NULL, >+ "failed: realloc should fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ /* ..as well as allocs */ >+ printf("==== talloc_size(pool, 1024)\n"); >+ l1 = talloc_size(pool, 1024); >+ torture_assert("memlimit", l1 == NULL, >+ "failed: alloc should fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ printf("==== talloc_free_children(pool)\n"); >+ talloc_free_children(pool); >+ >+ printf("==== talloc_set_memlimit(pool, 1024)\n"); >+ talloc_set_memlimit(pool, 1024); >+ >+ /* We should still be able to allocate up to the pool limit >+ because the memlimit only applies to new heap allocations */ >+ printf("==== talloc_size(pool, 9*1024)\n"); >+ l1 = talloc_size(pool, 9*1024); >+ torture_assert("memlimit", l1 != NULL, >+ "failed: alloc should not fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ l1 = talloc_size(pool, 1024); >+ torture_assert("memlimit", l1 == NULL, >+ "failed: alloc should fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ >+ printf("==== talloc_free_children(pool)\n"); >+ talloc_free_children(pool); >+ >+ printf("==== talloc_set_memlimit(pool, 10*1024)\n"); >+ talloc_set_memlimit(pool, 10*1024); >+ >+ printf("==== talloc_size(pool, 1024)\n"); >+ l1 = talloc_size(pool, 1024); >+ torture_assert("memlimit", l1 != NULL, >+ "failed: alloc should not fail due to memory limit\n"); >+ >+ talloc_report_full(pool, stdout); >+ > talloc_free(pool); > talloc_free(root); > printf("success: memlimit\n"); >-- >2.25.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 14540
:
16296
|
16297
|
16298
|
16313
|
16314