[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