From 87f563c2308b761bc9c42caf5ab240746004e908 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Nov 2008 01:58:56 -0800 Subject: Start factoring out the inheritance differences. Jeremy. --- source3/lib/secdesc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/secdesc.c b/source3/lib/secdesc.c index 2987306066..94d249564f 100644 --- a/source3/lib/secdesc.c +++ b/source3/lib/secdesc.c @@ -574,8 +574,7 @@ NTSTATUS se_create_child_secdesc(TALLOC_CTX *ctx, } *ppsd = make_sec_desc(ctx, SECURITY_DESCRIPTOR_REVISION_1, - SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT| - SEC_DESC_DACL_DEFAULTED, + SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT, owner_sid, group_sid, NULL, -- cgit From 8b4b5c3a92be83e99d9177b04f0da56f610025de Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Nov 2008 18:53:00 -0800 Subject: Add wrapper str_list_make_v3() to replace the old S3 behavior of str_list_make(). From Dan Sledz : In samba 3.2 passing NULL or an empty string returned NULL. In master, it now returns a list of length 1 with the first string set to NULL (an empty list). Jeremy. --- source3/lib/debug.c | 2 +- source3/lib/util_str.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'source3/lib') diff --git a/source3/lib/debug.c b/source3/lib/debug.c index 986dff48d7..d64fcb66d9 100644 --- a/source3/lib/debug.c +++ b/source3/lib/debug.c @@ -472,7 +472,7 @@ bool debug_parse_levels(const char *params_str) if (AllowDebugChange == False) return True; - params = str_list_make(talloc_tos(), params_str, NULL); + params = str_list_make_v3(talloc_tos(), params_str, NULL); if (debug_parse_params(params)) { debug_dump_status(5); diff --git a/source3/lib/util_str.c b/source3/lib/util_str.c index 046ce61ea3..fde4f825e8 100644 --- a/source3/lib/util_str.c +++ b/source3/lib/util_str.c @@ -2532,3 +2532,19 @@ char *escape_shell_string(const char *src) *dest++ = '\0'; return ret; } + +/*************************************************** + Wrapper for str_list_make() to restore the s3 behavior. + In samba 3.2 passing NULL or an empty string returned NULL. + + In master, it now returns a list of length 1 with the first string set + to NULL (an empty list) +***************************************************/ + +char **str_list_make_v3(TALLOC_CTX *mem_ctx, const char *string, const char *sep) +{ + if (!string || !*string) { + return NULL; + } + return str_list_make(mem_ctx, string, sep); +} -- cgit From 8962be69c700224983af4effd2cd086f7f5800b0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Nov 2008 20:48:13 -0800 Subject: Make us clean under valgrind --leak-check=full by using talloc_autofree_context() instead of NULL. Remove the code in memcache that does a TALLOC_FREE on stored pointers. That's a disaster waiting to happen. If you're storing talloc'ed pointers, you can't know their lifecycle and they should be deleted when their parent context is deleted, so freeing them at some arbitrary point later will be a double-free. Jeremy. --- source3/lib/memcache.c | 26 -------------------------- source3/lib/util.c | 6 +++--- source3/lib/util_pw.c | 2 +- 3 files changed, 4 insertions(+), 30 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index 9c892fedfa..d586f707fa 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -40,37 +40,11 @@ struct memcache { static void memcache_element_parse(struct memcache_element *e, DATA_BLOB *key, DATA_BLOB *value); -static bool memcache_is_talloc(enum memcache_number n) -{ - bool result; - - switch (n) { - case GETPWNAM_CACHE: - case PDB_GETPWSID_CACHE: - case SINGLETON_CACHE_TALLOC: - result = true; - break; - default: - result = false; - break; - } - - return result; -} - static int memcache_destructor(struct memcache *cache) { struct memcache_element *e, *next; for (e = cache->mru; e != NULL; e = next) { next = e->next; - if (memcache_is_talloc((enum memcache_number)e->n) - && (e->valuelength == sizeof(void *))) { - DATA_BLOB key, value; - void *ptr; - memcache_element_parse(e, &key, &value); - memcpy(&ptr, value.data, sizeof(ptr)); - TALLOC_FREE(ptr); - } SAFE_FREE(e); } return 0; diff --git a/source3/lib/util.c b/source3/lib/util.c index 820cf376be..5007fb72ef 100644 --- a/source3/lib/util.c +++ b/source3/lib/util.c @@ -1497,7 +1497,7 @@ uid_t nametouid(const char *name) char *p; uid_t u; - pass = getpwnam_alloc(NULL, name); + pass = getpwnam_alloc(talloc_autofree_context(), name); if (pass) { u = pass->pw_uid; TALLOC_FREE(pass); @@ -2255,8 +2255,8 @@ char *myhostname(void) static char *ret; if (ret == NULL) { /* This is cached forever so - * use NULL talloc ctx. */ - ret = talloc_get_myname(NULL); + * use talloc_autofree_context() ctx. */ + ret = talloc_get_myname(talloc_autofree_context()); } return ret; } diff --git a/source3/lib/util_pw.c b/source3/lib/util_pw.c index c0d37f1094..e0dbc97f00 100644 --- a/source3/lib/util_pw.c +++ b/source3/lib/util_pw.c @@ -57,7 +57,7 @@ struct passwd *getpwnam_alloc(TALLOC_CTX *mem_ctx, const char *name) return NULL; } - cached = tcopy_passwd(NULL, temp); + cached = tcopy_passwd(talloc_autofree_context(), temp); if (cached == NULL) { /* * Just don't add this into the cache, ignore the failure -- cgit From 5a2feed9dc4e7d28dee619a6941aa49be76fb298 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Nov 2008 23:29:20 -0800 Subject: If we didn't inherit any ACE's the ACE pointer should be NULL. Jeremy. --- source3/lib/secdesc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/secdesc.c b/source3/lib/secdesc.c index 94d249564f..de547d815f 100644 --- a/source3/lib/secdesc.c +++ b/source3/lib/secdesc.c @@ -563,15 +563,17 @@ NTSTATUS se_create_child_secdesc(TALLOC_CTX *ctx, } /* Create child security descriptor to return */ - - new_dacl = make_sec_acl(ctx, + if (new_ace_list_ndx) { + new_dacl = make_sec_acl(ctx, NT4_ACL_REVISION, new_ace_list_ndx, new_ace_list); - if (!new_dacl) { - return NT_STATUS_NO_MEMORY; + if (!new_dacl) { + return NT_STATUS_NO_MEMORY; + } } + *ppsd = make_sec_desc(ctx, SECURITY_DESCRIPTOR_REVISION_1, SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT, -- cgit From 813bf8b4f463199b7c2d3cddab7056b8a68a0b70 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 8 Nov 2008 22:57:57 -0800 Subject: Fix a subtle logic bug in the adaption of se_create_child_secdesc(), pass RAW-ACL inheritance tests. Only access masks for SD get/set left to fix. Jeremy. --- source3/lib/secdesc.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'source3/lib') diff --git a/source3/lib/secdesc.c b/source3/lib/secdesc.c index de547d815f..df85336603 100644 --- a/source3/lib/secdesc.c +++ b/source3/lib/secdesc.c @@ -546,6 +546,9 @@ NTSTATUS se_create_child_secdesc(TALLOC_CTX *ctx, ptrustee = creator; new_flags |= SEC_ACE_FLAG_INHERIT_ONLY; + } else if (container && + !(ace->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT)) { + ptrustee = &ace->trustee; } init_sec_ace(new_ace, ptrustee, ace->type, -- cgit From 1b41f670fc40ac583f546440c2a683e94eb05caf Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 14 Nov 2008 12:49:18 +0100 Subject: sys_pwnam doesn't return talloced memory, so don't mix up the returned struct. (cherry picked from commit eb99923991960e53bd150ac8f1d818cb746101b4) --- source3/lib/util_pw.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/util_pw.c b/source3/lib/util_pw.c index e0dbc97f00..e138273e8b 100644 --- a/source3/lib/util_pw.c +++ b/source3/lib/util_pw.c @@ -59,10 +59,7 @@ struct passwd *getpwnam_alloc(TALLOC_CTX *mem_ctx, const char *name) cached = tcopy_passwd(talloc_autofree_context(), temp); if (cached == NULL) { - /* - * Just don't add this into the cache, ignore the failure - */ - return temp; + return NULL; } memcache_add_talloc(NULL, GETPWNAM_CACHE, data_blob_string_const_null(name), -- cgit From 5a210cc552f92459dc05c44a435acbcbbe6db9e7 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 14 Nov 2008 13:13:40 +0100 Subject: Rename some variables in getpwnam_alloc() for clarity --- source3/lib/util_pw.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/util_pw.c b/source3/lib/util_pw.c index e138273e8b..c9b26f0a4f 100644 --- a/source3/lib/util_pw.c +++ b/source3/lib/util_pw.c @@ -44,27 +44,28 @@ void flush_pwnam_cache(void) struct passwd *getpwnam_alloc(TALLOC_CTX *mem_ctx, const char *name) { - struct passwd *temp, *cached; + struct passwd *pw, *for_cache; - temp = (struct passwd *)memcache_lookup_talloc( + pw = (struct passwd *)memcache_lookup_talloc( NULL, GETPWNAM_CACHE, data_blob_string_const_null(name)); - if (temp != NULL) { - return tcopy_passwd(mem_ctx, temp); + if (pw != NULL) { + return tcopy_passwd(mem_ctx, pw); } - temp = sys_getpwnam(name); - if (temp == NULL) { + pw = sys_getpwnam(name); + if (pw == NULL) { return NULL; } - cached = tcopy_passwd(talloc_autofree_context(), temp); - if (cached == NULL) { + for_cache = tcopy_passwd(talloc_autofree_context(), pw); + if (for_cache == NULL) { return NULL; } - memcache_add_talloc(NULL, GETPWNAM_CACHE, data_blob_string_const_null(name), - cached); - return tcopy_passwd(mem_ctx, temp); + memcache_add_talloc(NULL, GETPWNAM_CACHE, + data_blob_string_const_null(name), for_cache); + + return tcopy_passwd(mem_ctx, pw); } struct passwd *getpwuid_alloc(TALLOC_CTX *mem_ctx, uid_t uid) -- cgit From f50ad767505cc8847f5f574767b664b57326e468 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 13 Nov 2008 23:50:19 +0100 Subject: Actually finish memcache_add_talloc This fixes a memleak found by Martin Zielinski . Thanks for looking closely! Volker (cherry picked from commit a31a84a078100819809e6d40dbc3df207a50a0b2) --- source3/lib/memcache.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) (limited to 'source3/lib') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index d586f707fa..1951b4abf9 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -40,6 +40,24 @@ struct memcache { static void memcache_element_parse(struct memcache_element *e, DATA_BLOB *key, DATA_BLOB *value); +static bool memcache_is_talloc(enum memcache_number n) +{ + bool result; + + switch (n) { + case GETPWNAM_CACHE: + case PDB_GETPWSID_CACHE: + case SINGLETON_CACHE_TALLOC: + result = true; + break; + default: + result = false; + break; + } + + return result; +} + static int memcache_destructor(struct memcache *cache) { struct memcache_element *e, *next; @@ -188,6 +206,16 @@ static void memcache_delete_element(struct memcache *cache, } DLIST_REMOVE(cache->mru, e); + if (memcache_is_talloc(e->n)) { + DATA_BLOB cache_key, cache_value; + void *ptr; + + memcache_element_parse(e, &cache_key, &cache_value); + SMB_ASSERT(cache_value.length == sizeof(ptr)); + memcpy(&ptr, cache_value.data, sizeof(ptr)); + TALLOC_FREE(ptr); + } + cache->size -= memcache_element_size(e->keylength, e->valuelength); SAFE_FREE(e); @@ -250,6 +278,12 @@ void memcache_add(struct memcache *cache, enum memcache_number n, memcache_element_parse(e, &cache_key, &cache_value); if (value.length <= cache_value.length) { + if (memcache_is_talloc(e->n)) { + void *ptr; + SMB_ASSERT(cache_value.length == sizeof(ptr)); + memcpy(&ptr, cache_value.data, sizeof(ptr)); + TALLOC_FREE(ptr); + } /* * We can reuse the existing record */ @@ -308,7 +342,8 @@ void memcache_add(struct memcache *cache, enum memcache_number n, void memcache_add_talloc(struct memcache *cache, enum memcache_number n, DATA_BLOB key, void *ptr) { - memcache_add(cache, n, key, data_blob_const(&ptr, sizeof(ptr))); + void *p = talloc_move(cache, &ptr); + memcache_add(cache, n, key, data_blob_const(&p, sizeof(p))); } void memcache_flush(struct memcache *cache, enum memcache_number n) -- cgit From 3c98d5bd987358b1cbeb81fa8db37b97492cf0cc Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 14 Nov 2008 13:42:54 +0100 Subject: Make memcache_add_talloc NULL out the source pointer This is an orthogonality measure to make clear this pointer now belongs to the cache. (cherry picked from commit e6080c6e87d6fe3995b121a772bf3f6343fa666f) --- source3/lib/memcache.c | 14 ++++++++++++-- source3/lib/util_pw.c | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) (limited to 'source3/lib') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index 1951b4abf9..eaff15deda 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -340,9 +340,19 @@ void memcache_add(struct memcache *cache, enum memcache_number n, } void memcache_add_talloc(struct memcache *cache, enum memcache_number n, - DATA_BLOB key, void *ptr) + DATA_BLOB key, void **pptr) { - void *p = talloc_move(cache, &ptr); + void **ptr = (void **)pptr; + void *p; + + if (cache == NULL) { + cache = global_cache; + } + if (cache == NULL) { + return; + } + + p = talloc_move(cache, ptr); memcache_add(cache, n, key, data_blob_const(&p, sizeof(p))); } diff --git a/source3/lib/util_pw.c b/source3/lib/util_pw.c index c9b26f0a4f..b0baa12c3e 100644 --- a/source3/lib/util_pw.c +++ b/source3/lib/util_pw.c @@ -63,7 +63,7 @@ struct passwd *getpwnam_alloc(TALLOC_CTX *mem_ctx, const char *name) } memcache_add_talloc(NULL, GETPWNAM_CACHE, - data_blob_string_const_null(name), for_cache); + data_blob_string_const_null(name), &for_cache); return tcopy_passwd(mem_ctx, pw); } -- cgit From df21095ce8981827767e8646b6aefb053beb29a8 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 15 Nov 2008 00:50:33 +0100 Subject: Attempt to fix the build I have no idea how this could have happened. Probably done a make and make test in a different tree than the one I have done the push from. Sorry. Volker --- source3/lib/memcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source3/lib') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index eaff15deda..606d72ab5a 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -340,7 +340,7 @@ void memcache_add(struct memcache *cache, enum memcache_number n, } void memcache_add_talloc(struct memcache *cache, enum memcache_number n, - DATA_BLOB key, void **pptr) + DATA_BLOB key, void *pptr) { void **ptr = (void **)pptr; void *p; -- cgit From 9a3be6f0f8e120797a02fa1be60b51812cfd86f5 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 8 Nov 2008 16:48:20 +0100 Subject: Move cli_trans_oob to lib/util.c Rename it to trans_oob, it will be used in the server routines. --- source3/lib/util.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'source3/lib') diff --git a/source3/lib/util.c b/source3/lib/util.c index 5007fb72ef..074b523ae0 100644 --- a/source3/lib/util.c +++ b/source3/lib/util.c @@ -2878,6 +2878,25 @@ int this_is_smp(void) #endif } +/**************************************************************** + Check if offset/length fit into bufsize. Should probably be + merged with is_offset_safe, but this would require a rewrite + of lanman.c. Later :-) +****************************************************************/ + +bool trans_oob(uint32_t bufsize, uint32_t offset, uint32_t length) +{ + if ((offset + length < offset) || (offset + length < length)) { + /* wrap */ + return true; + } + if ((offset > bufsize) || (offset + length > bufsize)) { + /* overflow */ + return true; + } + return false; +} + /**************************************************************** Check if an offset into a buffer is safe. If this returns True it's safe to indirect into the byte at -- cgit