[PATCH] Fix new Coverity IDs

Jeremy Allison jra at samba.org
Wed Jan 11 21:07:55 UTC 2017


On Wed, Jan 11, 2017 at 11:47:06AM -0800, Jeremy Allison wrote:
> On Wed, Jan 11, 2017 at 11:07:18AM -0800, Jeremy Allison wrote:
> > 1501                 num_rids = talloc_array_length(rids);
> > 
> > so 'rids' is already initialized to NULL, but we then
> > call talloc_array_length(rids) without checking if
> > the return was NULL. This works (I think), but goes
> > via:
> > 
> > #define talloc_array_length(ctx) (talloc_get_size(ctx)/sizeof(*ctx))
> > 
> > and talloc_get_size() is:
> > 
> > _PUBLIC_ size_t talloc_get_size(const void *context)
> > {
> >         struct talloc_chunk *tc;
> > 
> >         if (context == NULL) {
> >                 context = null_context;
> >         }
> >         if (context == NULL) {
> >                 return 0;
> >         }
> > 
> >         tc = talloc_chunk_from_ptr(context);
> > 
> >         return tc->size;
> > }
> > 
> > - which uses null_context - which can change what it returns
> > if someone ever calls talloc_enable_null_tracking() !!!!!
> 
> Never mind. As Volker just pointed out to me on the phone,
> null_context = _talloc_named_const(NULL, 0, "null_context"),
> which will always return zero size anyway. Still,
> that code shouldn't be in talloc_get_size().
> 
> Patch(es) to follow !

Here the are. Please review and push if happy !

(Finally fixed the to: address for metze, sorry).

Jeremy.
-------------- next part --------------
From 69f8dfb789c8297b4f7a91fe85f85ec9bf974786 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 11 Jan 2017 11:48:25 -0800
Subject: [PATCH 1/2] lib: talloc: Make it clear that talloc_get_size(NULL)
 returns 0.

This *isn't* a behavior change, as the previous code could potentially
return the size of null_context, which (currently) is defined as
a named talloc region of ZERO size, but this makes it very clear
what the ABI behavior should be.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 lib/talloc/talloc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 279021c137a..8bdd4b6653a 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -2739,9 +2739,6 @@ _PUBLIC_ size_t talloc_get_size(const void *context)
 	struct talloc_chunk *tc;
 
 	if (context == NULL) {
-		context = null_context;
-	}
-	if (context == NULL) {
 		return 0;
 	}
 
-- 
2.11.0.390.gc69c2f50cf-goog


From e76f09215511c99520dc9d6a7d656ddf2f0e42ba Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 11 Jan 2017 11:52:44 -0800
Subject: [PATCH 2/2] winbind: Fix CID 1398534 Dereference before null check

Make all query_user_list backends consistent.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/winbindd/winbindd_ads.c  | 7 ++++---
 source3/winbindd/winbindd_samr.c | 2 --
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index b14f21e3644..3653e517bf6 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -299,8 +299,6 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
 	LDAPMessage *msg = NULL;
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
 
-	*prids = NULL;
-
 	DEBUG(3,("ads: query_user_list\n"));
 
 	if ( !winbindd_can_contact_domain( domain ) ) {
@@ -375,7 +373,10 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
 	}
 
 	rids = talloc_realloc(mem_ctx, rids, uint32_t, count);
-	*prids = rids;
+
+	if (prids) {
+		*prids = rids;
+	}
 
 	status = NT_STATUS_OK;
 
diff --git a/source3/winbindd/winbindd_samr.c b/source3/winbindd/winbindd_samr.c
index 224f1058348..84ed8a21858 100644
--- a/source3/winbindd/winbindd_samr.c
+++ b/source3/winbindd/winbindd_samr.c
@@ -176,8 +176,6 @@ static NTSTATUS sam_query_user_list(struct winbindd_domain *domain,
 
 	DEBUG(3,("samr_query_user_list\n"));
 
-	*prids = NULL;
-
 	tmp_ctx = talloc_stackframe();
 	if (tmp_ctx == NULL) {
 		return NT_STATUS_NO_MEMORY;
-- 
2.11.0.390.gc69c2f50cf-goog



More information about the samba-technical mailing list