[PATCH] Improve talloc performance
Jeremy Allison
jra at samba.org
Thu Jun 30 22:43:28 UTC 2016
On Fri, Jul 01, 2016 at 12:39:15AM +0200, Michael Adam wrote:
> On 2016-06-30 at 15:16 -0700, Jeremy Allison wrote:
> > On Thu, Jun 30, 2016 at 11:52:22PM +0200, Michael Adam wrote:
> > >
> > > From a superficial glance, the patches look clean,
> > > straight and reasonably-sized. But one cosmetic comment:
> > > you seem to have dropped the actual patch authorship.
> > > At least the singoff and review tags seem to imply that. :-)
> >
> > Didn't think so - this is Andrew's original work
> > split up (and somewhat renamed) by me. I thought
> > I'd got that correct. What did I mess up ?
>
> The reworked patches all carry
> "From: Jeremy Allison <jra at samba.org>"
> so if I read that right, they'll all have you as author.
>
> If you want Andrew to be listed as the author, you'd need to do
>
> git commit --author="Andrew Bartlett <abartlet at samba.org>" ...
>
> or similar, etc.
Ah, got it - thanks ! Updated set attached.
-------------- 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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: Andrew Bartlett <abartlet 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