[PATCH] memset_s() and talloc_set_secure()
Andreas Schneider
asn at samba.org
Fri Oct 12 12:14:03 UTC 2018
On Thursday, 11 October 2018 21:06:20 CEST Jeremy Allison wrote:
> On Fri, Oct 12, 2018 at 07:57:31AM +1300, Andrew Bartlett via samba-
technical wrote:
> > This still doesn't handle realloc correctly. The current code there
> > only handles the non-pool case when ALWAYS_REALLOC is defined, which
> > never is.
> >
> > You need to force all talloc_realloc() calls on secret memory though
> > this path, as realloc() won't let you zero the memory after, and unlike
> > the talloc header (which we overstamp and then restore) you don't do
> > something similar with the content.
> >
> > talloc_realloc() is rare, and talloc_set_secret() is even more rare. I
> > suggest just banning the combination and checking for things like
> > talloc_realloc() and talloc_asprintf_append() in the small number of
> > impacted code paths manually.
>
> +1 on this design decision. This is a very narrow talloc use
> case and it would be better to restrict it to avoid internal
> complexity.
>
> We've made this mistake before, let's not make it again.
> All the talloc code around talloc_set_memlimit() is a
> horrible increase in complexity for little (not at all
> in Samba) used functionality.
I've improved the code with metze today. talloc_realloc() is also working with
talloc_keep_secret() now. I've also added a test for it.
See attached patchset.
Please review, we need more eyes on this.
Thanks,
Andreas
--
Andreas Schneider asn at samba.org
Samba Team www.samba.org
GPG-ID: 8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D
-------------- next part --------------
>From 1585a0f9d569513f845dd17933e90a766f28ca22 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 10 Oct 2018 16:05:46 +0200
Subject: [PATCH 1/9] replace: Add memset_s() if not available
See https://en.cppreference.com/w/c/string/byte/memset
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/replace/replace.c | 31 +++++++++++++++++++++++++++++++
lib/replace/replace.h | 8 ++++++++
lib/replace/wscript | 17 +++++++++++++++++
3 files changed, 56 insertions(+)
diff --git a/lib/replace/replace.c b/lib/replace/replace.c
index dc81e9cd5ab..113137c2992 100644
--- a/lib/replace/replace.c
+++ b/lib/replace/replace.c
@@ -947,3 +947,34 @@ void rep_setproctitle_init(int argc, char *argv[], char *envp[])
{
}
#endif
+
+#ifndef HAVE_MEMSET_S
+# ifndef RSIZE_MAX
+# define RSIZE_MAX (SIZE_MAX >> 1)
+# endif
+
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count)
+{
+ if (dest == NULL) {
+ return EINVAL;
+ }
+
+ if (destsz > RSIZE_MAX ||
+ count > RSIZE_MAX ||
+ count > destsz) {
+ return ERANGE;
+ }
+
+#if defined(HAVE_MEMSET_EXPLICIT)
+ memset_explicit(dest, destsz, ch, count);
+#else /* HAVE_MEMSET_EXPLICIT */
+ memset(dest, ch, count);
+# if defined(HAVE_GCC_VOLATILE_MEMORY_PROTECTION)
+ /* See http://llvm.org/bugs/show_bug.cgi?id=15495 */
+ __asm__ volatile("" : : "g"(dest) : "memory");
+# endif /* HAVE_GCC_VOLATILE_MEMORY_PROTECTION */
+#endif /* HAVE_MEMSET_EXPLICIT */
+
+ return 0;
+}
+#endif /* HAVE_MEMSET_S */
diff --git a/lib/replace/replace.h b/lib/replace/replace.h
index 626d3053029..de4e20c4454 100644
--- a/lib/replace/replace.h
+++ b/lib/replace/replace.h
@@ -36,6 +36,9 @@
#include <standards.h>
#endif
+/* Needs to be defined before std*.h and string*.h are included */
+#define __STDC_WANT_LIB_EXT1__ 1
+
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
@@ -925,6 +928,11 @@ void rep_setproctitle(const char *fmt, ...) PRINTF_ATTRIBUTE(1, 2);
void rep_setproctitle_init(int argc, char *argv[], char *envp[]);
#endif
+#ifndef HAVE_MEMSET_S
+#define memset_s rep_memset_s
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count);
+#endif
+
#ifndef FALL_THROUGH
# ifdef HAVE_FALLTHROUGH_ATTRIBUTE
# define FALL_THROUGH __attribute__ ((fallthrough))
diff --git a/lib/replace/wscript b/lib/replace/wscript
index 8e2873424be..8adfffe9584 100644
--- a/lib/replace/wscript
+++ b/lib/replace/wscript
@@ -195,6 +195,23 @@ def configure(conf):
'socket nsl', checklibc=True,
headers='sys/socket.h netinet/in.h arpa/inet.h netdb.h')
+ conf.CHECK_FUNCS('memset_s memset_explicit')
+
+ conf.CHECK_CODE('''
+ #include <string.h>
+
+ int main(void)
+ {
+ char buf[] = "This is some content";
+ memset(buf, '\0', sizeof(buf)); __asm__ volatile("" : : "g"(&buf) : "memory");
+ return 0;
+ }
+ ''',
+ define='HAVE_GCC_VOLATILE_MEMORY_PROTECTION',
+ addmain=False,
+ msg='Checking for volatile memory protection',
+ local_include=False)
+
# Some old Linux systems have broken header files and
# miss the IPV6_V6ONLY define in netinet/in.h,
# but have it in linux/in6.h.
--
2.19.0
>From 3a3aecdd6f712f779d0477fea97aaa7a2db39206 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 10 Oct 2018 16:09:32 +0200
Subject: [PATCH 2/9] s3:lib:popt: Use memset_s() to burn string
Signed-off-by: Andreas Schneider <asn at samba.org>
---
source3/lib/popt_common_cmdline.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/lib/popt_common_cmdline.c b/source3/lib/popt_common_cmdline.c
index fe23f84c547..971234257e6 100644
--- a/source3/lib/popt_common_cmdline.c
+++ b/source3/lib/popt_common_cmdline.c
@@ -217,7 +217,7 @@ void popt_burn_cmdline_password(int argc, char *argv[])
p = strchr_m(p, '%');
if (p != NULL) {
- memset(p, '\0', strlen(p));
+ memset_s(p, strlen(p), '\0', strlen(p));
}
found = false;
}
--
2.19.0
>From 0cebacf84abfadb7d68e4b261cd87b7c5221957b Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 11:58:26 +0200
Subject: [PATCH 3/9] talloc: Fix alignment issues for casting pointers
warning: cast from 'char *' to 'struct talloc_chunk *' increases required
alignment from 1 to 8
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 54be63495ae..aee70a659b1 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -317,6 +317,11 @@ struct talloc_chunk {
struct talloc_pool_hdr *pool;
};
+union talloc_chunk_cast_u {
+ uint8_t *ptr;
+ struct talloc_chunk *chunk;
+};
+
/* 16 byte alignment seems to keep everyone happy */
#define TC_ALIGN16(s) (((s)+15)&~15)
#define TC_HDR_SIZE TC_ALIGN16(sizeof(struct talloc_chunk))
@@ -611,16 +616,33 @@ struct talloc_pool_hdr {
size_t poolsize;
};
+union talloc_pool_hdr_cast_u {
+ uint8_t *ptr;
+ struct talloc_pool_hdr *hdr;
+};
+
#define TP_HDR_SIZE TC_ALIGN16(sizeof(struct talloc_pool_hdr))
static inline struct talloc_pool_hdr *talloc_pool_from_chunk(struct talloc_chunk *c)
{
- return (struct talloc_pool_hdr *)((char *)c - TP_HDR_SIZE);
+ union talloc_pool_hdr_cast_u tphc;
+ union talloc_chunk_cast_u tcc;
+
+ tcc.chunk = c;
+ tphc.ptr = tcc.ptr - TP_HDR_SIZE;
+
+ return tphc.hdr;
}
static inline struct talloc_chunk *talloc_chunk_from_pool(struct talloc_pool_hdr *h)
{
- return (struct talloc_chunk *)((char *)h + TP_HDR_SIZE);
+ union talloc_pool_hdr_cast_u tphc;
+ union talloc_chunk_cast_u tcc;
+
+ tphc.hdr = h;
+ tcc.ptr = tphc.ptr + TP_HDR_SIZE;
+
+ return tcc.chunk;
}
static inline void *tc_pool_end(struct talloc_pool_hdr *pool_hdr)
@@ -668,6 +690,7 @@ 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;
+ union talloc_chunk_cast_u tcc;
size_t space_left;
struct talloc_chunk *result;
size_t chunk_size;
@@ -698,7 +721,8 @@ static inline struct talloc_chunk *tc_alloc_pool(struct talloc_chunk *parent,
return NULL;
}
- result = (struct talloc_chunk *)((char *)pool_hdr->end + prefix_len);
+ tcc.ptr = ((uint8_t *)pool_hdr->end) + prefix_len;
+ result = tcc.chunk;
#if defined(DEVELOPER) && defined(VALGRIND_MAKE_MEM_UNDEFINED)
VALGRIND_MAKE_MEM_UNDEFINED(pool_hdr->end, chunk_size);
@@ -750,7 +774,8 @@ static inline void *__talloc_with_prefix(const void *context,
}
if (tc == NULL) {
- char *ptr;
+ uint8_t *ptr = NULL;
+ union talloc_chunk_cast_u tcc;
/*
* Only do the memlimit check/update on actual allocation.
@@ -764,7 +789,8 @@ static inline void *__talloc_with_prefix(const void *context,
if (unlikely(ptr == NULL)) {
return NULL;
}
- tc = (struct talloc_chunk *)(ptr + prefix_len);
+ tcc.ptr = ptr + prefix_len;
+ tc = tcc.chunk;
tc->flags = talloc_magic;
tc->pool = NULL;
--
2.19.0
>From ea37c4b90934a5c8f36440a3da84eb5b6924cf26 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Oct 2018 07:54:41 +0200
Subject: [PATCH 4/9] talloc: Add support to keep talloc chunks secret
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 26 +++++++++++++++++++-------
lib/talloc/talloc.h | 20 ++++++++++++++++++++
2 files changed, 39 insertions(+), 7 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index aee70a659b1..f609231793b 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -65,12 +65,13 @@
#define TALLOC_FLAG_LOOP 0x02
#define TALLOC_FLAG_POOL 0x04 /* This is a talloc pool */
#define TALLOC_FLAG_POOLMEM 0x08 /* This is allocated in a pool */
+#define TALLOC_FLAG_SECRET 0x10
/*
* Bits above this are random, used to make it harder to fake talloc
* headers during an attack. Try not to change this without good reason.
*/
-#define TALLOC_FLAG_MASK 0x0F
+#define TALLOC_FLAG_MASK 0x1F
#define TALLOC_MAGIC_REFERENCE ((const char *)1)
@@ -143,10 +144,10 @@ static struct {
* double-free logic to still work
*/
#define TC_INVALIDATE_FULL_FILL_CHUNK(_tc) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size; \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -167,11 +168,11 @@ static struct {
} while (0)
#define TC_INVALIDATE_SHRINK_FILL_CHUNK(_tc, _new_size) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size - (_new_size); \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
_fptr += (_new_size); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -193,11 +194,11 @@ static struct {
} while (0)
#define TC_UNDEFINE_SHRINK_FILL_CHUNK(_tc, _new_size) do { \
- if (unlikely(talloc_fill.enabled)) { \
+ if (unlikely(talloc_fill.enabled || (_tc)->flags & TALLOC_FLAG_SECRET)) { \
size_t _flen = (_tc)->size - (_new_size); \
char *_fptr = (char *)TC_PTR_FROM_CHUNK(_tc); \
_fptr += (_new_size); \
- memset(_fptr, talloc_fill.fill_value, _flen); \
+ memset_s(_fptr, _flen, talloc_fill.fill_value, _flen); \
} \
} while (0)
@@ -973,6 +974,9 @@ static int talloc_reference_destructor(struct talloc_reference_handle *handle)
static inline void _tc_set_name_const(struct talloc_chunk *tc,
const char *name)
{
+ if (unlikely(tc->flags & TALLOC_FLAG_SECRET)) {
+ return;
+ }
tc->name = name;
}
@@ -1529,6 +1533,14 @@ _PUBLIC_ const char *talloc_set_name(const void *ptr, const char *fmt, ...)
return name;
}
+_PUBLIC_ void _talloc_keep_secret(const void *ptr, const char *name)
+{
+ struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
+
+ _tc_set_name_const(tc, name);
+
+ tc->flags |= TALLOC_FLAG_SECRET;
+}
/*
create a named talloc pointer. Any talloc pointer can be named, and
diff --git a/lib/talloc/talloc.h b/lib/talloc/talloc.h
index 7372df16fe8..b6f6ddf23fd 100644
--- a/lib/talloc/talloc.h
+++ b/lib/talloc/talloc.h
@@ -392,6 +392,26 @@ void *_talloc_steal_loc(const void *new_ctx, const void *ptr, const char *locati
*/
const char *talloc_set_name(const void *ptr, const char *fmt, ...) PRINTF_ATTRIBUTE(2,3);
+#ifdef DOXYGEN
+/**
+ * @brief Keep the memory secret when freeing.
+ *
+ * This can be used to define memory as secret. For example memory which holds
+ * passwords or other tokens. The memory will be zeroed before is being freed.
+ *
+ * If you duplicate memory, e.g. using talloc_strdup() or talloc_asprintf() you
+ * need to call talloc_keep_secret() on the newly allocated memory too!
+ *
+ * @param[in] ptr The talloc chunk to mark as secure.
+ *
+ * @warning Do not use this in combination with talloc_realloc().
+ */
+void talloc_keep_secret(const void *ptr);
+#else
+#define talloc_keep_secret(ptr) _talloc_keep_secret(ptr, #ptr);
+void _talloc_keep_secret(const void *ptr, const char *name);
+#endif
+
#ifdef DOXYGEN
/**
* @brief Change a talloc chunk's parent.
--
2.19.0
>From 176dd8b8bb5a7a90d52ea1f4be0dcd02e96afc17 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 11:16:15 +0200
Subject: [PATCH 5/9] talloc: Don't return the size of secret chunks
Signed-off-by: Andreas Schneider <asn 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 f609231793b..ed862c61681 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -2152,7 +2152,7 @@ static inline size_t _talloc_total_mem_internal(const void *ptr,
return tc->limit->cur_size;
}
- if (tc->flags & TALLOC_FLAG_LOOP) {
+ if (tc->flags & (TALLOC_FLAG_LOOP|TALLOC_FLAG_SECRET)) {
return 0;
}
--
2.19.0
>From bd541a6f50033cf8586d6add505ddc3b24609448 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Oct 2018 08:25:22 +0200
Subject: [PATCH 6/9] talloc: Add test for talloc_keep_secret()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/tests/test_talloc_keep_secret.c | 97 ++++++++++++++++++++++
lib/talloc/wscript | 6 ++
selftest/tests.py | 6 ++
3 files changed, 109 insertions(+)
create mode 100644 lib/talloc/tests/test_talloc_keep_secret.c
diff --git a/lib/talloc/tests/test_talloc_keep_secret.c b/lib/talloc/tests/test_talloc_keep_secret.c
new file mode 100644
index 00000000000..3fd743aec94
--- /dev/null
+++ b/lib/talloc/tests/test_talloc_keep_secret.c
@@ -0,0 +1,97 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <string.h>
+#include <talloc.h>
+
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count);
+
+int rep_memset_s(void *dest, size_t destsz, int ch, size_t count)
+{
+ check_expected_ptr(dest);
+ check_expected(destsz);
+ check_expected(ch);
+ check_expected(count);
+
+ return 0;
+}
+
+static void test_talloc_keep_secret(void ** state)
+{
+ TALLOC_CTX *pool = NULL;
+ char *ptr1 = NULL;
+ char *ptr2 = NULL;
+ const char *ptr1_talloc_name = NULL;
+ size_t ptr1_size;
+ size_t total_size;
+ size_t i;
+
+ pool = talloc_pool(NULL, 256);
+ assert_non_null(pool);
+
+ ptr1 = talloc_strdup(pool, "secret");
+ assert_non_null(ptr1);
+ assert_string_equal(ptr1, "secret");
+
+ talloc_keep_secret(ptr1);
+
+ ptr1_talloc_name = talloc_get_name(ptr1);
+ assert_string_equal(ptr1_talloc_name, "ptr1");
+
+ ptr1_size = talloc_get_size(ptr1);
+ assert_int_equal(ptr1_size, strlen(ptr1) + 1);
+
+ total_size = talloc_total_size(ptr1);
+ assert_int_equal(total_size, 0);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, strlen(ptr1) + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, strlen(ptr1) + 1);
+
+ talloc_free(ptr1);
+
+ ptr2 = talloc_size(pool, ptr1_size);
+ assert_ptr_equal(ptr1, ptr2);
+
+ for (i = 1; i < ptr1_size; i++) {
+ assert_int_not_equal(ptr2[0], ptr2[i]);
+ }
+
+ talloc_free(pool);
+}
+
+static void test_talloc_keep_secret_validate_memset(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, strlen(password) + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, strlen(password) + 1);
+
+ talloc_free(mem_ctx);
+}
+
+int main(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_talloc_keep_secret),
+ cmocka_unit_test(test_talloc_keep_secret_validate_memset),
+ };
+
+ cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/lib/talloc/wscript b/lib/talloc/wscript
index a5841b46e4e..1e047e94635 100644
--- a/lib/talloc/wscript
+++ b/lib/talloc/wscript
@@ -139,6 +139,12 @@ def build(bld):
private_library=private_library,
manpages='man/talloc.3')
+ # We only check for cmocka in the Samba tree
+ bld.SAMBA_BINARY('test_talloc_keep_secret',
+ 'tests/test_talloc_keep_secret.c',
+ 'talloc cmocka',
+ install=False)
+
if not bld.CONFIG_SET('USING_SYSTEM_PYTALLOC_UTIL'):
for env in bld.gen_python_environments(['PKGCONFIGDIR']):
name = bld.pyembed_libname('pytalloc-util')
diff --git a/selftest/tests.py b/selftest/tests.py
index daa3bb7390c..48738b5c2f3 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -41,6 +41,8 @@ have_man_pages_support = ("XSLTPROC_MANPAGES" in config_hash)
with_pam = ("WITH_PAM" in config_hash)
pam_wrapper_so_path = config_hash["LIBPAM_WRAPPER_SO_PATH"]
pam_set_items_so_path = config_hash["PAM_SET_ITEMS_SO_PATH"]
+with_internal_talloc = ("TALLOC_BUILD_VERSION_MAJOR" in config_hash)
+with_memset_s = ("HAVE_MEMSET_S" in config_hash)
planpythontestsuite("none", "samba.tests.source", py3_compatible=True)
if have_man_pages_support:
@@ -193,6 +195,10 @@ plantestsuite("samba.unittests.lib_util_modules", "none",
plantestsuite("samba.unittests.smb1cli_session", "none",
[os.path.join(bindir(), "default/libcli/smb/test_smb1cli_session")])
+if with_internal_talloc and not with_memset_s:
+ plantestsuite("talloc.keep_secret", "none",
+ [os.path.join(bindir(), "default/lib/talloc/test_talloc_keep_secret")])
+
plantestsuite("samba.unittests.tldap", "none",
[os.path.join(bindir(), "default/source3/test_tldap")])
plantestsuite("samba.unittests.rfc1738", "none",
--
2.19.0
>From 6280792c45fe91c9b054bde5da775cfb61afc28c Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 11:07:07 +0200
Subject: [PATCH 7/9] talloc: Handle TALLOC_FLAG_SECRET in talloc_realloc()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/talloc.c | 63 ++++++++++++++++++++++++---------------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index ed862c61681..402ac5b56b4 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -54,11 +54,6 @@
#include <valgrind.h>
#endif
-/* use this to force every realloc to change the pointer, to stress test
- code that might not cope */
-#define ALWAYS_REALLOC 0
-
-
#define MAX_TALLOC_SIZE 0x10000000
#define TALLOC_FLAG_FREE 0x01
@@ -1861,7 +1856,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
pool_hdr = tc->pool;
}
-#if (ALWAYS_REALLOC == 0)
/* don't shrink if we have less than 1k to gain */
if (size < tc->size && tc->limit == NULL) {
if (pool_hdr) {
@@ -1881,6 +1875,9 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
* testing a lot :-(.
*
* That is why we only mark memory as undefined here.
+ *
+ * In case of secret memory, TC_UNDEFINE_SHRINK_CHUNK
+ * will zero it.
*/
TC_UNDEFINE_SHRINK_CHUNK(tc, size);
@@ -1895,7 +1892,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
*/
return ptr;
}
-#endif
/*
* by resetting magic we catch users of the old memory
@@ -1914,32 +1910,42 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
*/
_talloc_chunk_set_free(tc, NULL);
-#if ALWAYS_REALLOC
- if (pool_hdr) {
- new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
- pool_hdr->object_count--;
+ if (tc->flags & TALLOC_FLAG_SECRET) {
+ if (pool_hdr != NULL) {
+ 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;
+ if (new_ptr == NULL) {
+ new_ptr = malloc(TC_HDR_SIZE+size);
+ malloced = true;
+ new_size = size;
+ }
+
+ if (new_ptr != NULL) {
+ memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
+ TC_INVALIDATE_FULL_CHUNK(tc);
+ }
+ } else {
+ /*
+ * We're doing malloc then free here, so record the
+ * difference.
+ */
+ old_size = tc->size;
new_size = size;
+ new_ptr = malloc(size + TC_HDR_SIZE);
+ if (new_ptr) {
+ memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE);
+ if (tc->flags & TALLOC_FLAG_SECRET) {
+ uint8_t *zero_ptr = (uint8_t *)TC_PTR_FROM_CHUNK(tc);
+ memset_s(zero_ptr, old_size, talloc_fill.fill_value, old_size);
+ }
+ free(tc);
+ }
}
- if (new_ptr) {
- memcpy(new_ptr, tc, MIN(tc->size,size) + TC_HDR_SIZE);
- TC_INVALIDATE_FULL_CHUNK(tc);
- }
- } else {
- /* We're doing malloc then free here, so record the difference. */
- old_size = tc->size;
- new_size = size;
- new_ptr = malloc(size + TC_HDR_SIZE);
- if (new_ptr) {
- memcpy(new_ptr, tc, MIN(tc->size, size) + TC_HDR_SIZE);
- free(tc);
- }
+ goto got_new_ptr;
}
-#else
+
if (pool_hdr) {
struct talloc_chunk *pool_tc;
void *next_tc = tc_next_chunk(tc);
@@ -2054,7 +2060,6 @@ _PUBLIC_ void *_talloc_realloc(const void *context, void *ptr, size_t size, cons
new_ptr = realloc(tc, size + TC_HDR_SIZE);
}
got_new_ptr:
-#endif
if (unlikely(!new_ptr)) {
/*
* Ok, this is a strange spot. We have to put back
--
2.19.0
>From cbe8788b6bed321b9485fd870ff96256773b09bb Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Fri, 12 Oct 2018 12:47:34 +0200
Subject: [PATCH 8/9] talloc: Add keep secret test for talloc_realloc()
Signed-off-by: Andreas Schneider <asn at samba.org>
---
lib/talloc/tests/test_talloc_keep_secret.c | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/lib/talloc/tests/test_talloc_keep_secret.c b/lib/talloc/tests/test_talloc_keep_secret.c
index 3fd743aec94..6a273fbb11f 100644
--- a/lib/talloc/tests/test_talloc_keep_secret.c
+++ b/lib/talloc/tests/test_talloc_keep_secret.c
@@ -84,11 +84,46 @@ static void test_talloc_keep_secret_validate_memset(void **state)
talloc_free(mem_ctx);
}
+static void test_talloc_keep_secret_realloc(void **state)
+{
+ TALLOC_CTX *mem_ctx = NULL;
+ char *password = NULL;
+ size_t pwdlen;
+
+ mem_ctx = talloc_new(NULL);
+ assert_non_null(mem_ctx);
+
+ password = talloc_strdup(mem_ctx, "secret");
+ assert_non_null(password);
+ talloc_keep_secret(password);
+
+ pwdlen = strlen(password);
+
+ expect_string(rep_memset_s, dest, "secret");
+ expect_value(rep_memset_s, destsz, pwdlen + 1);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen + 1);
+
+ password = talloc_realloc(mem_ctx, password, char, strlen(password) + 2);
+ assert_non_null(password);
+
+ password[pwdlen + 1] = '\0';
+ password[pwdlen] = '1';
+
+ expect_string(rep_memset_s, dest, "secret1");
+ expect_value(rep_memset_s, destsz, pwdlen + 2);
+ expect_value(rep_memset_s, ch, (int)'\0');
+ expect_value(rep_memset_s, count, pwdlen + 2);
+
+ talloc_free(mem_ctx);
+}
+
int main(void)
{
const struct CMUnitTest tests[] = {
cmocka_unit_test(test_talloc_keep_secret),
cmocka_unit_test(test_talloc_keep_secret_validate_memset),
+ cmocka_unit_test(test_talloc_keep_secret_realloc),
};
cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
--
2.19.0
>From 477b5d5fe873dee37433c6ff7d5643eaa809c361 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Thu, 11 Oct 2018 08:46:26 +0200
Subject: [PATCH 9/9] ABI
It is strange that you get an error if you bump the minor version.
Because this is what you need to do if you add a function to a library.
The patch level is only bumped if there is a code change which doesn't
change the ABI.
The new version should be 2.2.0
---
lib/talloc/ABI/pytalloc-util-2.1.15.sigs | 16 ++++++
lib/talloc/ABI/talloc-2.1.15.sigs | 66 ++++++++++++++++++++++++
lib/talloc/wscript | 2 +-
3 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 lib/talloc/ABI/pytalloc-util-2.1.15.sigs
create mode 100644 lib/talloc/ABI/talloc-2.1.15.sigs
diff --git a/lib/talloc/ABI/pytalloc-util-2.1.15.sigs b/lib/talloc/ABI/pytalloc-util-2.1.15.sigs
new file mode 100644
index 00000000000..9d4d4d142c8
--- /dev/null
+++ b/lib/talloc/ABI/pytalloc-util-2.1.15.sigs
@@ -0,0 +1,16 @@
+_pytalloc_check_type: int (PyObject *, const char *)
+_pytalloc_get_mem_ctx: TALLOC_CTX *(PyObject *)
+_pytalloc_get_ptr: void *(PyObject *)
+_pytalloc_get_type: void *(PyObject *, const char *)
+pytalloc_BaseObject_PyType_Ready: int (PyTypeObject *)
+pytalloc_BaseObject_check: int (PyObject *)
+pytalloc_BaseObject_size: size_t (void)
+pytalloc_CObject_FromTallocPtr: PyObject *(void *)
+pytalloc_Check: int (PyObject *)
+pytalloc_GenericObject_reference_ex: PyObject *(TALLOC_CTX *, void *)
+pytalloc_GenericObject_steal_ex: PyObject *(TALLOC_CTX *, void *)
+pytalloc_GetBaseObjectType: PyTypeObject *(void)
+pytalloc_GetObjectType: PyTypeObject *(void)
+pytalloc_reference_ex: PyObject *(PyTypeObject *, TALLOC_CTX *, void *)
+pytalloc_steal: PyObject *(PyTypeObject *, void *)
+pytalloc_steal_ex: PyObject *(PyTypeObject *, TALLOC_CTX *, void *)
diff --git a/lib/talloc/ABI/talloc-2.1.15.sigs b/lib/talloc/ABI/talloc-2.1.15.sigs
new file mode 100644
index 00000000000..972448138f0
--- /dev/null
+++ b/lib/talloc/ABI/talloc-2.1.15.sigs
@@ -0,0 +1,66 @@
+_talloc: void *(const void *, size_t)
+_talloc_array: void *(const void *, size_t, unsigned int, const char *)
+_talloc_free: int (void *, const char *)
+_talloc_get_type_abort: void *(const void *, const char *, const char *)
+_talloc_keep_secret: void (const void *, const char *)
+_talloc_memdup: void *(const void *, const void *, size_t, const char *)
+_talloc_move: void *(const void *, const void *)
+_talloc_pooled_object: void *(const void *, size_t, const char *, unsigned int, size_t)
+_talloc_realloc: void *(const void *, void *, size_t, const char *)
+_talloc_realloc_array: void *(const void *, void *, size_t, unsigned int, const char *)
+_talloc_reference_loc: void *(const void *, const void *, const char *)
+_talloc_set_destructor: void (const void *, int (*)(void *))
+_talloc_steal_loc: void *(const void *, const void *, const char *)
+_talloc_zero: void *(const void *, size_t, const char *)
+_talloc_zero_array: void *(const void *, size_t, unsigned int, const char *)
+talloc_asprintf: char *(const void *, const char *, ...)
+talloc_asprintf_append: char *(char *, const char *, ...)
+talloc_asprintf_append_buffer: char *(char *, const char *, ...)
+talloc_autofree_context: void *(void)
+talloc_check_name: void *(const void *, const char *)
+talloc_disable_null_tracking: void (void)
+talloc_enable_leak_report: void (void)
+talloc_enable_leak_report_full: void (void)
+talloc_enable_null_tracking: void (void)
+talloc_enable_null_tracking_no_autofree: void (void)
+talloc_find_parent_byname: void *(const void *, const char *)
+talloc_free_children: void (void *)
+talloc_get_name: const char *(const void *)
+talloc_get_size: size_t (const void *)
+talloc_increase_ref_count: int (const void *)
+talloc_init: void *(const char *, ...)
+talloc_is_parent: int (const void *, const void *)
+talloc_named: void *(const void *, size_t, const char *, ...)
+talloc_named_const: void *(const void *, size_t, const char *)
+talloc_parent: void *(const void *)
+talloc_parent_name: const char *(const void *)
+talloc_pool: void *(const void *, size_t)
+talloc_realloc_fn: void *(const void *, void *, size_t)
+talloc_reference_count: size_t (const void *)
+talloc_reparent: void *(const void *, const void *, const void *)
+talloc_report: void (const void *, FILE *)
+talloc_report_depth_cb: void (const void *, int, int, void (*)(const void *, int, int, int, void *), void *)
+talloc_report_depth_file: void (const void *, int, int, FILE *)
+talloc_report_full: void (const void *, FILE *)
+talloc_set_abort_fn: void (void (*)(const char *))
+talloc_set_log_fn: void (void (*)(const char *))
+talloc_set_log_stderr: void (void)
+talloc_set_memlimit: int (const void *, size_t)
+talloc_set_name: const char *(const void *, const char *, ...)
+talloc_set_name_const: void (const void *, const char *)
+talloc_show_parents: void (const void *, FILE *)
+talloc_strdup: char *(const void *, const char *)
+talloc_strdup_append: char *(char *, const char *)
+talloc_strdup_append_buffer: char *(char *, const char *)
+talloc_strndup: char *(const void *, const char *, size_t)
+talloc_strndup_append: char *(char *, const char *, size_t)
+talloc_strndup_append_buffer: char *(char *, const char *, size_t)
+talloc_test_get_magic: int (void)
+talloc_total_blocks: size_t (const void *)
+talloc_total_size: size_t (const void *)
+talloc_unlink: int (const void *, void *)
+talloc_vasprintf: char *(const void *, const char *, va_list)
+talloc_vasprintf_append: char *(char *, const char *, va_list)
+talloc_vasprintf_append_buffer: char *(char *, const char *, va_list)
+talloc_version_major: int (void)
+talloc_version_minor: int (void)
diff --git a/lib/talloc/wscript b/lib/talloc/wscript
index 1e047e94635..a48a4ad2585 100644
--- a/lib/talloc/wscript
+++ b/lib/talloc/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'talloc'
-VERSION = '2.1.14'
+VERSION = '2.1.15'
import os
import sys
--
2.19.0
More information about the samba-technical
mailing list