[PATCH] Improve talloc performance
Jeremy Allison
jra at samba.org
Thu Jun 30 17:48:11 UTC 2016
On Wed, Jun 29, 2016 at 04:55:23PM -0700, Jeremy Allison wrote:
> On Fri, Jun 17, 2016 at 05:12:12PM -0700, Jeremy Allison wrote:
> > On Fri, Jun 17, 2016 at 09:50:27PM +1200, Andrew Bartlett wrote:
> > > Attached is a patch to improve talloc performance, by calling
> > > talloc_chunk_from_ptr() less often. I'm impressed by how much can be
> > > gained!
> > >
> > > Please carefully review!
> >
> > I'm going through this right now, but one thing
> > that has to be done is to split this into a set of smaller
> > changesets as the chunks are too large to carefully parse
> > mentally (at least for me). Too much change per patchset.
>
> OK, split this into an 11-patch set, with each change
> being somewhat clearer and smaller (IMHO of course :-).
>
> Your original patchset had one error (missing setting
> the .name in the replacement for talloc_vasprintf(),
> and a couple of places where you added code, then removed
> it again.
>
> I'm still running tests on it at the moment, but at
> least it compiles :-).
>
> I'll post for your review once I've gotten it past
> testing !
OK, here it is. Passes testing and valgrind. Keeps
the performance boost.
The first patch renames internal functions that
take a 'struct talloc_chunk *' from talloc_XXX()
to tc_XXX(). Subsequent patches keep that naming
scheme. Doing this I find it much easier to tell
the functions that take a talloc_chunk apart from
the ones that take the void * talloc pointer. Has
no external effect on the ABI of course.
The rest of them are your patch split into easily
digestible changes.
Next time, try and do this yourself to save me a
lot of work (although I'd have to understand all
the changes anyway of course :-). Small micro-patches
the do only one thing really make this work easier.
Please review and push if happy !
Jeremy.
-------------- next part --------------
From c9de97afe1edee8e3e7258fd61ac602326d2a9be Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 16:25:30 -0700
Subject: [PATCH 01/11] lib: talloc: Rename talloc_XXX() internal functions
that take a 'struct talloc_chunk *' to tc_XXX().
We will be adding more and it ensures a consistent naming scheme.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 48d2033..7b9fc7a 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -250,7 +250,7 @@ static inline void talloc_memlimit_grow(struct talloc_memlimit *limit,
size_t size);
static inline void talloc_memlimit_shrink(struct talloc_memlimit *limit,
size_t size);
-static inline void talloc_memlimit_update_on_free(struct talloc_chunk *tc);
+static inline void tc_memlimit_update_on_free(struct talloc_chunk *tc);
static inline void _talloc_set_name_const(const void *ptr, const char *name);
@@ -572,7 +572,7 @@ static inline void tc_invalidate_pool(struct talloc_pool_hdr *pool_hdr)
Allocate from a pool
*/
-static inline struct talloc_chunk *talloc_alloc_pool(struct talloc_chunk *parent,
+static inline struct talloc_chunk *tc_alloc_pool(struct talloc_chunk *parent,
size_t size, size_t prefix_len)
{
struct talloc_pool_hdr *pool_hdr = NULL;
@@ -651,7 +651,7 @@ static inline void *__talloc_with_prefix(const void *context, size_t size,
limit = ptc->limit;
}
- tc = talloc_alloc_pool(ptc, TC_HDR_SIZE+size, prefix_len);
+ tc = tc_alloc_pool(ptc, TC_HDR_SIZE+size, prefix_len);
}
if (tc == NULL) {
@@ -905,7 +905,7 @@ _PUBLIC_ void *_talloc_reference_loc(const void *context, const void *ptr, const
static void *_talloc_steal_internal(const void *new_ctx, const void *ptr);
-static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
+static inline void _tc_free_poolmem(struct talloc_chunk *tc,
const char *location)
{
struct talloc_pool_hdr *pool;
@@ -956,15 +956,15 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
pool_tc->name = location;
if (pool_tc->flags & TALLOC_FLAG_POOLMEM) {
- _talloc_free_poolmem(pool_tc, location);
+ _tc_free_poolmem(pool_tc, location);
} else {
/*
- * The talloc_memlimit_update_on_free()
+ * The tc_memlimit_update_on_free()
* call takes into account the
* prefix TP_HDR_SIZE allocated before
* the pool talloc_chunk.
*/
- talloc_memlimit_update_on_free(pool_tc);
+ tc_memlimit_update_on_free(pool_tc);
TC_INVALIDATE_FULL_CHUNK(pool_tc);
free(pool);
}
@@ -987,7 +987,7 @@ static inline void _talloc_free_poolmem(struct talloc_chunk *tc,
*/
}
-static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
+static inline void _tc_free_children_internal(struct talloc_chunk *tc,
void *ptr,
const char *location);
@@ -1069,7 +1069,7 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
tc->flags |= TALLOC_FLAG_LOOP;
- _talloc_free_children_internal(tc, ptr, location);
+ _tc_free_children_internal(tc, ptr, location);
tc->flags |= TALLOC_FLAG_FREE;
@@ -1106,11 +1106,11 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
}
if (tc->flags & TALLOC_FLAG_POOLMEM) {
- _talloc_free_poolmem(tc, location);
+ _tc_free_poolmem(tc, location);
return 0;
}
- talloc_memlimit_update_on_free(tc);
+ tc_memlimit_update_on_free(tc);
TC_INVALIDATE_FULL_CHUNK(tc);
free(ptr_to_free);
@@ -1506,7 +1506,7 @@ _PUBLIC_ void *talloc_init(const char *fmt, ...)
return ptr;
}
-static inline void _talloc_free_children_internal(struct talloc_chunk *tc,
+static inline void _tc_free_children_internal(struct talloc_chunk *tc,
void *ptr,
const char *location)
{
@@ -1568,7 +1568,7 @@ _PUBLIC_ void talloc_free_children(void *ptr)
}
}
- _talloc_free_children_internal(tc, ptr, __location__);
+ _tc_free_children_internal(tc, ptr, __location__);
/* .. so we put it back after all other children have been freed */
if (tc_name) {
@@ -1742,7 +1742,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
#if ALWAYS_REALLOC
if (pool_hdr) {
- new_ptr = talloc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
+ new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
pool_hdr->object_count--;
if (new_ptr == NULL) {
@@ -1859,7 +1859,7 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
}
}
- new_ptr = talloc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
+ new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
if (new_ptr == NULL) {
new_ptr = malloc(TC_HDR_SIZE+size);
@@ -1870,7 +1870,7 @@ _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);
- _talloc_free_poolmem(tc, __location__ "_talloc_realloc");
+ _tc_free_poolmem(tc, __location__ "_talloc_realloc");
}
}
else {
@@ -2772,7 +2772,7 @@ static inline bool talloc_memlimit_check(struct talloc_memlimit *limit, size_t s
/*
Update memory limits when freeing a talloc_chunk.
*/
-static void talloc_memlimit_update_on_free(struct talloc_chunk *tc)
+static void tc_memlimit_update_on_free(struct talloc_chunk *tc)
{
size_t limit_shrink_size;
--
2.8.0.rc3.226.g39d4020
From 5cf7c5011f1dcb35f6a10842a94985327cac1897 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 17 Jun 2016 16:58:34 -0700
Subject: [PATCH 02/11] lib: talloc: Change __talloc_with_prefix() to return a
struct talloc_chunk *.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 7b9fc7a..3873fa6 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -625,8 +625,10 @@ static inline struct talloc_chunk *tc_alloc_pool(struct talloc_chunk *parent,
/*
Allocate a bit of memory as a child of an existing pointer
*/
-static inline void *__talloc_with_prefix(const void *context, size_t size,
- size_t prefix_len)
+static inline void *__talloc_with_prefix(const void *context,
+ size_t size,
+ size_t prefix_len,
+ struct talloc_chunk **tc_ret)
{
struct talloc_chunk *tc = NULL;
struct talloc_memlimit *limit = NULL;
@@ -700,12 +702,14 @@ static inline void *__talloc_with_prefix(const void *context, size_t size,
tc->next = tc->prev = tc->parent = NULL;
}
+ *tc_ret = tc;
return TC_PTR_FROM_CHUNK(tc);
}
static inline void *__talloc(const void *context, size_t size)
{
- return __talloc_with_prefix(context, size, 0);
+ struct talloc_chunk *tc;
+ return __talloc_with_prefix(context, size, 0, &tc);
}
/*
@@ -718,13 +722,12 @@ static inline void *_talloc_pool(const void *context, size_t size)
struct talloc_pool_hdr *pool_hdr;
void *result;
- result = __talloc_with_prefix(context, size, TP_HDR_SIZE);
+ result = __talloc_with_prefix(context, size, TP_HDR_SIZE, &tc);
if (unlikely(result == NULL)) {
return NULL;
}
- tc = talloc_chunk_from_ptr(result);
pool_hdr = talloc_pool_from_chunk(tc);
tc->flags |= TALLOC_FLAG_POOL;
--
2.8.0.rc3.226.g39d4020
From 769dab2ca294a65fb7622b127c591319b155ae50 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 17 Jun 2016 17:06:52 -0700
Subject: [PATCH 03/11] lib: talloc: Change __talloc() to return a struct
talloc_chunk *.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 3873fa6..1c09fb6 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -706,10 +706,11 @@ static inline void *__talloc_with_prefix(const void *context,
return TC_PTR_FROM_CHUNK(tc);
}
-static inline void *__talloc(const void *context, size_t size)
+static inline void *__talloc(const void *context,
+ size_t size,
+ struct talloc_chunk **tc)
{
- struct talloc_chunk *tc;
- return __talloc_with_prefix(context, size, 0, &tc);
+ return __talloc_with_prefix(context, size, 0, tc);
}
/*
@@ -864,8 +865,9 @@ static inline void _talloc_set_name_const(const void *ptr, const char *name)
static inline void *_talloc_named_const(const void *context, size_t size, const char *name)
{
void *ptr;
+ struct talloc_chunk *tc;
- ptr = __talloc(context, size);
+ ptr = __talloc(context, size, &tc);
if (unlikely(ptr == NULL)) {
return NULL;
}
@@ -1398,8 +1400,9 @@ _PUBLIC_ void *talloc_named(const void *context, size_t size, const char *fmt, .
va_list ap;
void *ptr;
const char *name;
+ struct talloc_chunk *tc;
- ptr = __talloc(context, size);
+ ptr = __talloc(context, size, &tc);
if (unlikely(ptr == NULL)) return NULL;
va_start(ap, fmt);
@@ -1493,8 +1496,9 @@ _PUBLIC_ void *talloc_init(const char *fmt, ...)
va_list ap;
void *ptr;
const char *name;
+ struct talloc_chunk *tc;
- ptr = __talloc(NULL, 0);
+ ptr = __talloc(NULL, 0, &tc);
if (unlikely(ptr == NULL)) return NULL;
va_start(ap, fmt);
@@ -1588,7 +1592,8 @@ _PUBLIC_ void talloc_free_children(void *ptr)
*/
_PUBLIC_ void *_talloc(const void *context, size_t size)
{
- return __talloc(context, size);
+ struct talloc_chunk *tc;
+ return __talloc(context, size, &tc);
}
/*
@@ -2298,8 +2303,9 @@ _PUBLIC_ void *_talloc_memdup(const void *t, const void *p, size_t size, const c
static inline char *__talloc_strlendup(const void *t, const char *p, size_t len)
{
char *ret;
+ struct talloc_chunk *tc;
- ret = (char *)__talloc(t, len + 1);
+ ret = (char *)__talloc(t, len + 1, &tc);
if (unlikely(!ret)) return NULL;
memcpy(ret, p, len);
@@ -2436,6 +2442,7 @@ _PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
int len;
char *ret;
va_list ap2;
+ struct talloc_chunk *tc;
char buf[1024];
/* this call looks strange, but it makes it work on older solaris boxes */
@@ -2446,7 +2453,7 @@ _PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
return NULL;
}
- ret = (char *)__talloc(t, len+1);
+ ret = (char *)__talloc(t, len+1, &tc);
if (unlikely(!ret)) return NULL;
if (len < sizeof(buf)) {
--
2.8.0.rc3.226.g39d4020
From 9b29c70f8123537bfc0e9d83317c10c91b60dc81 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 17 Jun 2016 20:40:56 -0700
Subject: [PATCH 04/11] lib: talloc: Change _talloc_set_name_const() to
_tc_set_name_const()
First argument is now struct talloc_chunk *tc.
Ensure all callers pass correct talloc chunk from given pointer.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 1c09fb6..3e844c4 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -252,7 +252,8 @@ static inline void talloc_memlimit_shrink(struct talloc_memlimit *limit,
size_t size);
static inline void tc_memlimit_update_on_free(struct talloc_chunk *tc);
-static inline void _talloc_set_name_const(const void *ptr, const char *name);
+static inline void _tc_set_name_const(struct talloc_chunk *tc,
+ const char *name);
typedef int (*talloc_destructor_t)(void *);
@@ -807,7 +808,7 @@ _PUBLIC_ void *_talloc_pooled_object(const void *ctx,
pool_hdr->end = ((char *)pool_hdr->end + TC_ALIGN16(type_size));
- _talloc_set_name_const(ret, type_name);
+ _tc_set_name_const(tc, type_name);
return ret;
overflow:
@@ -853,9 +854,9 @@ static int talloc_reference_destructor(struct talloc_reference_handle *handle)
more efficient way to add a name to a pointer - the name must point to a
true string constant
*/
-static inline void _talloc_set_name_const(const void *ptr, const char *name)
+static inline void _tc_set_name_const(struct talloc_chunk *tc,
+ const char *name)
{
- struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
tc->name = name;
}
@@ -872,7 +873,7 @@ static inline void *_talloc_named_const(const void *context, size_t size, const
return NULL;
}
- _talloc_set_name_const(ptr, name);
+ _tc_set_name_const(tc, name);
return ptr;
}
@@ -1371,7 +1372,8 @@ static inline const char *talloc_set_name_v(const void *ptr, const char *fmt, va
struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
tc->name = talloc_vasprintf(ptr, fmt, ap);
if (likely(tc->name)) {
- _talloc_set_name_const(tc->name, ".name");
+ _tc_set_name_const(talloc_chunk_from_ptr(tc->name),
+ ".name");
}
return tc->name;
}
@@ -1601,7 +1603,7 @@ _PUBLIC_ void *_talloc(const void *context, size_t size)
*/
_PUBLIC_ void talloc_set_name_const(const void *ptr, const char *name)
{
- _talloc_set_name_const(ptr, name);
+ _tc_set_name_const(talloc_chunk_from_ptr(ptr), name);
}
/*
@@ -1920,7 +1922,7 @@ got_new_ptr:
}
tc->size = size;
- _talloc_set_name_const(TC_PTR_FROM_CHUNK(tc), name);
+ _tc_set_name_const(tc, name);
return TC_PTR_FROM_CHUNK(tc);
}
@@ -2311,7 +2313,7 @@ static inline char *__talloc_strlendup(const void *t, const char *p, size_t len)
memcpy(ret, p, len);
ret[len] = 0;
- _talloc_set_name_const(ret, ret);
+ _tc_set_name_const(tc, ret);
return ret;
}
@@ -2345,7 +2347,7 @@ static inline char *__talloc_strlendup_append(char *s, size_t slen,
memcpy(&ret[slen], a, alen);
ret[slen+alen] = 0;
- _talloc_set_name_const(ret, ret);
+ _tc_set_name_const(talloc_chunk_from_ptr(ret), ret);
return ret;
}
@@ -2464,7 +2466,7 @@ _PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
va_end(ap2);
}
- _talloc_set_name_const(ret, ret);
+ _tc_set_name_const(talloc_chunk_from_ptr(ret), ret);
return ret;
}
@@ -2516,7 +2518,7 @@ static inline char *__talloc_vaslenprintf_append(char *s, size_t slen,
vsnprintf(s + slen, alen + 1, fmt, ap2);
va_end(ap2);
- _talloc_set_name_const(s, s);
+ _tc_set_name_const(talloc_chunk_from_ptr(s), s);
return s;
}
--
2.8.0.rc3.226.g39d4020
From 60e73ab418fea687bbcd813e57431c6ad147ae12 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 17 Jun 2016 20:49:24 -0700
Subject: [PATCH 05/11] lib: talloc: Add _vasprintf_tc() which returns the
struct talloc_chunk *, not the talloc'ed pointer.
Define talloc_vasprintf() in terms of _vasprintf_tc().
We will use _vasprintf_tc() internally later.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 3e844c4..6cc67e5 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -254,6 +254,9 @@ static inline void tc_memlimit_update_on_free(struct talloc_chunk *tc);
static inline void _tc_set_name_const(struct talloc_chunk *tc,
const char *name);
+static struct talloc_chunk *_vasprintf_tc(const void *t,
+ const char *fmt,
+ va_list ap);
typedef int (*talloc_destructor_t)(void *);
@@ -2439,7 +2442,9 @@ _PUBLIC_ char *talloc_strndup_append_buffer(char *s, const char *a, size_t n)
#endif
#endif
-_PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
+static struct talloc_chunk *_vasprintf_tc(const void *t,
+ const char *fmt,
+ va_list ap)
{
int len;
char *ret;
@@ -2466,8 +2471,17 @@ _PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
va_end(ap2);
}
- _tc_set_name_const(talloc_chunk_from_ptr(ret), ret);
- return ret;
+ _tc_set_name_const(tc, ret);
+ return tc;
+}
+
+_PUBLIC_ char *talloc_vasprintf(const void *t, const char *fmt, va_list ap)
+{
+ struct talloc_chunk *tc = _vasprintf_tc(t, fmt, ap);
+ if (tc == NULL) {
+ return NULL;
+ }
+ return TC_PTR_FROM_CHUNK(tc);
}
--
2.8.0.rc3.226.g39d4020
From 41bbf50b15cbd3fcd501c4a0c4eb068cd39b251d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 23 Jun 2016 17:17:20 -0700
Subject: [PATCH 06/11] lib: talloc: Rename talloc_set_name_v() to
tc_set_name_v(). Make it take a struct talloc_chunk *tc as the first
argument.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 6cc67e5..26cecee 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -1368,15 +1368,22 @@ _PUBLIC_ int talloc_unlink(const void *context, void *ptr)
/*
add a name to an existing pointer - va_list version
*/
-static inline const char *talloc_set_name_v(const void *ptr, const char *fmt, va_list ap) PRINTF_ATTRIBUTE(2,0);
+static inline const char *tc_set_name_v(struct talloc_chunk *tc,
+ const char *fmt,
+ va_list ap) PRINTF_ATTRIBUTE(2,0);
-static inline const char *talloc_set_name_v(const void *ptr, const char *fmt, va_list ap)
-{
- struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
- tc->name = talloc_vasprintf(ptr, fmt, ap);
- if (likely(tc->name)) {
- _tc_set_name_const(talloc_chunk_from_ptr(tc->name),
- ".name");
+static inline const char *tc_set_name_v(struct talloc_chunk *tc,
+ const char *fmt,
+ va_list ap)
+{
+ struct talloc_chunk *name_tc = _vasprintf_tc(TC_PTR_FROM_CHUNK(tc),
+ fmt,
+ ap);
+ if (likely(name_tc)) {
+ tc->name = TC_PTR_FROM_CHUNK(name_tc);
+ _tc_set_name_const(name_tc, ".name");
+ } else {
+ tc->name = NULL;
}
return tc->name;
}
@@ -1386,10 +1393,11 @@ static inline const char *talloc_set_name_v(const void *ptr, const char *fmt, va
*/
_PUBLIC_ const char *talloc_set_name(const void *ptr, const char *fmt, ...)
{
+ struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
const char *name;
va_list ap;
va_start(ap, fmt);
- name = talloc_set_name_v(ptr, fmt, ap);
+ name = tc_set_name_v(tc, fmt, ap);
va_end(ap);
return name;
}
@@ -1411,7 +1419,7 @@ _PUBLIC_ void *talloc_named(const void *context, size_t size, const char *fmt, .
if (unlikely(ptr == NULL)) return NULL;
va_start(ap, fmt);
- name = talloc_set_name_v(ptr, fmt, ap);
+ name = tc_set_name_v(tc, fmt, ap);
va_end(ap);
if (unlikely(name == NULL)) {
@@ -1507,7 +1515,7 @@ _PUBLIC_ void *talloc_init(const char *fmt, ...)
if (unlikely(ptr == NULL)) return NULL;
va_start(ap, fmt);
- name = talloc_set_name_v(ptr, fmt, ap);
+ name = tc_set_name_v(tc, fmt, ap);
va_end(ap);
if (unlikely(name == NULL)) {
--
2.8.0.rc3.226.g39d4020
From 1db21bfb92514e359c7d4e70e66cb6dface16d51 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 15:46:37 -0700
Subject: [PATCH 07/11] lib: talloc: Call talloc_chunk_from_ptr() less often in
__talloc_with_prefix()
Rename 'ptc' pointer to parent as it's re-used as
that name later in the function.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 26cecee..007b7a4 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -637,6 +637,7 @@ static inline void *__talloc_with_prefix(const void *context,
struct talloc_chunk *tc = NULL;
struct talloc_memlimit *limit = NULL;
size_t total_len = TC_HDR_SIZE + size + prefix_len;
+ struct talloc_chunk *parent = NULL;
if (unlikely(context == NULL)) {
context = null_context;
@@ -650,14 +651,14 @@ static inline void *__talloc_with_prefix(const void *context,
return NULL;
}
- if (context != NULL) {
- struct talloc_chunk *ptc = talloc_chunk_from_ptr(context);
+ if (likely(context != NULL)) {
+ parent = talloc_chunk_from_ptr(context);
- if (ptc->limit != NULL) {
- limit = ptc->limit;
+ if (parent->limit != NULL) {
+ limit = parent->limit;
}
- tc = tc_alloc_pool(ptc, TC_HDR_SIZE+size, prefix_len);
+ tc = tc_alloc_pool(parent, TC_HDR_SIZE+size, prefix_len);
}
if (tc == NULL) {
@@ -689,9 +690,7 @@ static inline void *__talloc_with_prefix(const void *context,
tc->name = NULL;
tc->refs = NULL;
- if (likely(context)) {
- struct talloc_chunk *parent = talloc_chunk_from_ptr(context);
-
+ if (likely(context != NULL)) {
if (parent->child) {
parent->child->parent = NULL;
tc->next = parent->child;
--
2.8.0.rc3.226.g39d4020
From 617ad3a9d8a62f010e4cd0bdc8c58537d301a25c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 16:41:52 -0700
Subject: [PATCH 08/11] lib: talloc: Rename the internals of
_talloc_free_internal() to _tc_free_internal().
Make it use a struct talloc_chunk *tc parameter. Define _talloc_free_internal()
in terms of _tc_free_internal().
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 50 +++++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 007b7a4..428fb8a 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -999,29 +999,16 @@ static inline void _tc_free_children_internal(struct talloc_chunk *tc,
void *ptr,
const char *location);
+static inline int _talloc_free_internal(void *ptr, const char *location);
+
/*
- internal talloc_free call
+ internal free call that takes a struct talloc_chunk *.
*/
-static inline int _talloc_free_internal(void *ptr, const char *location)
+static inline int _tc_free_internal(struct talloc_chunk *tc,
+ const char *location)
{
- struct talloc_chunk *tc;
void *ptr_to_free;
-
- if (unlikely(ptr == NULL)) {
- return -1;
- }
-
- /* possibly initialised the talloc fill value */
- if (unlikely(!talloc_fill.initialised)) {
- const char *fill = getenv(TALLOC_FILL_ENV);
- if (fill != NULL) {
- talloc_fill.enabled = true;
- talloc_fill.fill_value = strtoul(fill, NULL, 0);
- }
- talloc_fill.initialised = true;
- }
-
- tc = talloc_chunk_from_ptr(ptr);
+ void *ptr = TC_PTR_FROM_CHUNK(tc);
if (unlikely(tc->refs)) {
int is_child;
@@ -1125,6 +1112,31 @@ static inline int _talloc_free_internal(void *ptr, const char *location)
return 0;
}
+/*
+ internal talloc_free call
+*/
+static inline int _talloc_free_internal(void *ptr, const char *location)
+{
+ struct talloc_chunk *tc;
+
+ if (unlikely(ptr == NULL)) {
+ return -1;
+ }
+
+ /* possibly initialised the talloc fill value */
+ if (unlikely(!talloc_fill.initialised)) {
+ const char *fill = getenv(TALLOC_FILL_ENV);
+ if (fill != NULL) {
+ talloc_fill.enabled = true;
+ talloc_fill.fill_value = strtoul(fill, NULL, 0);
+ }
+ talloc_fill.initialised = true;
+ }
+
+ tc = talloc_chunk_from_ptr(ptr);
+ return _tc_free_internal(tc, location);
+}
+
static inline size_t _talloc_total_limit_size(const void *ptr,
struct talloc_memlimit *old_limit,
struct talloc_memlimit *new_limit);
--
2.8.0.rc3.226.g39d4020
From 53ec1072c4e33748b1d3d9a7e507cc9db9fc02b9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 16:44:50 -0700
Subject: [PATCH 09/11] lib: talloc: As _tc_free_internal() takes a struct
talloc_chunk *, add an extra paranoia check against destructor overwrite.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/talloc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 428fb8a..cd0ec3d 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -1034,6 +1034,20 @@ static inline int _tc_free_internal(struct talloc_chunk *tc,
if (unlikely(tc->destructor)) {
talloc_destructor_t d = tc->destructor;
+
+ /*
+ * Protect the destructor against some overwrite
+ * attacks, by explicitly checking it has the right
+ * magic here.
+ */
+ if (talloc_chunk_from_ptr(ptr) != tc) {
+ /*
+ * This can't actually happen, the
+ * call itself will panic.
+ */
+ TALLOC_ABORT("talloc_chunk_from_ptr failed!");
+ }
+
if (d == (talloc_destructor_t)-1) {
return -1;
}
--
2.8.0.rc3.226.g39d4020
From 01b195aba7e946e2118540ec4d926930a0a56c53 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 16:48:42 -0700
Subject: [PATCH 10/11] lib: talloc: As we have a struct talloc_chunk * in
_talloc_free_children_internal(), use it to call _tc_free_internal()
directly.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at 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 cd0ec3d..09318e9 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -1567,7 +1567,7 @@ static inline void _tc_free_children_internal(struct talloc_chunk *tc,
struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
if (p) new_parent = TC_PTR_FROM_CHUNK(p);
}
- if (unlikely(_talloc_free_internal(child, location) == -1)) {
+ if (unlikely(_tc_free_internal(tc->child, location) == -1)) {
if (talloc_parent_chunk(child) != tc) {
/*
* Destructor already reparented this child.
--
2.8.0.rc3.226.g39d4020
From d6fe4aa5724d55aa1de0321e29b115ff935c94c8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 29 Jun 2016 16:51:26 -0700
Subject: [PATCH 11/11] lib: talloc: Add check for destructor protection.
Signed-off-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
lib/talloc/testsuite.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index dd49df1..8fe5d90 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -1865,6 +1865,11 @@ static void test_magic_protection_abort(const char *reason)
}
}
+static int test_magic_protection_destructor(int *ptr)
+{
+ _exit(404); /* Not 42 */
+}
+
static bool test_magic_protection(void)
{
void *pool = talloc_pool(NULL, 1024);
@@ -1883,6 +1888,7 @@ static bool test_magic_protection(void)
pid = fork();
if (pid == 0) {
talloc_set_abort_fn(test_magic_protection_abort);
+ talloc_set_destructor(p2, test_magic_protection_destructor);
/*
* Simulate a security attack
--
2.8.0.rc3.226.g39d4020
More information about the samba-technical
mailing list