[PATCH] Two Coverity fixes and a few cleanups
Volker Lendecke
Volker.Lendecke at SerNet.DE
Mon Aug 17 10:31:00 UTC 2015
Hi!
Review&push appreciated!
Thanks,
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 816fb0c8e902ab15196e8bea90fbfbf14a1c9d52 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 16 Aug 2015 13:03:13 +0200
Subject: [PATCH 1/7] gensec: Fix CID 242642 Unchecked return value
Signed-off-by: Volker Lendecke <vl at samba.org>
---
auth/gensec/spnego.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/auth/gensec/spnego.c b/auth/gensec/spnego.c
index 85c70e1..c111b85 100644
--- a/auth/gensec/spnego.c
+++ b/auth/gensec/spnego.c
@@ -1184,6 +1184,7 @@ static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security
{
struct spnego_state *spnego_state = (struct spnego_state *)gensec_security->private_data;
DATA_BLOB out = data_blob_null;
+ bool ok;
*_out = data_blob_null;
@@ -1222,7 +1223,11 @@ static NTSTATUS gensec_spnego_update_out(struct gensec_security *gensec_security
/*
* truncate the buffer
*/
- data_blob_realloc(spnego_state, &out, spnego_state->out_max_length);
+ ok = data_blob_realloc(spnego_state, &out,
+ spnego_state->out_max_length);
+ if (!ok) {
+ return NT_STATUS_NO_MEMORY;
+ }
talloc_steal(out_mem_ctx, out.data);
*_out = out;
--
1.9.1
From b3eb611b3872261cfb9b15b954ee4f994c634ebc Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 16 Aug 2015 13:19:15 +0200
Subject: [PATCH 2/7] ctdb: Use talloc_report_str in ctdb
This fixes CID 1125620 Insecure temporary file
Signed-off-by: Volker Lendecke <vl at samba.org>
---
ctdb/server/ctdb_control.c | 40 +++++++++++++++-------------------------
ctdb/wscript | 4 ++--
2 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c
index 59b7d09..ee69270 100644
--- a/ctdb/server/ctdb_control.c
+++ b/ctdb/server/ctdb_control.c
@@ -24,6 +24,7 @@
#include "../include/ctdb_private.h"
#include "lib/util/dlinklist.h"
#include "lib/tdb_wrap/tdb_wrap.h"
+#include "lib/util/talloc_report.h"
struct ctdb_control_state {
@@ -40,34 +41,23 @@ struct ctdb_control_state {
*/
int32_t ctdb_dump_memory(struct ctdb_context *ctdb, TDB_DATA *outdata)
{
- /* dump to a file, then send the file as a blob */
- FILE *f;
- long fsize;
- f = tmpfile();
- if (f == NULL) {
- DEBUG(DEBUG_ERR,(__location__ " Unable to open tmpfile - %s\n", strerror(errno)));
- return -1;
- }
- talloc_report_full(NULL, f);
- fsize = ftell(f);
- if (fsize == -1) {
- DEBUG(DEBUG_ERR, (__location__ " Unable to get file size - %s\n",
- strerror(errno)));
- fclose(f);
+ char *report;
+ size_t reportlen;
+
+ report = talloc_report_str(outdata, NULL);
+ if (report == NULL) {
+ DEBUG(DEBUG_ERR,
+ (__location__ " talloc_report_str failed\n"));
return -1;
}
- rewind(f);
- outdata->dptr = talloc_size(outdata, fsize);
- if (outdata->dptr == NULL) {
- fclose(f);
- CTDB_NO_MEMORY(ctdb, outdata->dptr);
- }
- outdata->dsize = fread(outdata->dptr, 1, fsize, f);
- fclose(f);
- if (outdata->dsize != fsize) {
- DEBUG(DEBUG_ERR,(__location__ " Unable to read tmpfile\n"));
- return -1;
+ reportlen = talloc_get_size(report);
+
+ if (reportlen > 0) {
+ reportlen -= 1; /* strip trailing zero */
}
+
+ outdata->dptr = (uint8_t *)report;
+ outdata->dsize = reportlen;
return 0;
}
diff --git a/ctdb/wscript b/ctdb/wscript
index 06f86a3..8aa0374 100755
--- a/ctdb/wscript
+++ b/ctdb/wscript
@@ -348,7 +348,7 @@ def build(bld):
include/ctdb_private.h
include/ctdb_protocol.h
include/ctdb_typesafe_cb.h''',
- deps='replace popt talloc tevent tdb')
+ deps='replace popt talloc tevent tdb talloc_report')
bld.SAMBA_BINARY('ctdbd',
source='',
@@ -547,7 +547,7 @@ def build(bld):
bld.SAMBA_BINARY('ctdb_takeover_tests',
source='tests/src/ctdb_takeover_tests.c',
deps='''replace popt tdb tevent talloc ctdb-system
- samba-util tdb-wrap''' +
+ samba-util tdb-wrap talloc_report''' +
ib_deps,
includes='include include/internal',
install_path='${CTDB_TEST_LIBDIR}')
--
1.9.1
From 0a911ef7e48c1b499be13cc7dc3d757f2d3131b3 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 16 Aug 2015 20:25:31 +0200
Subject: [PATCH 3/7] ctdb: ctdb now needs talloc_report
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/wscript_build | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/util/wscript_build b/lib/util/wscript_build
index 9663bb0..e5c1a97 100755
--- a/lib/util/wscript_build
+++ b/lib/util/wscript_build
@@ -54,6 +54,13 @@ bld.SAMBA_LIBRARY('socket-blocking',
local_include=False,
private_library=True)
+bld.SAMBA_LIBRARY('talloc_report',
+ source='talloc_report.c',
+ local_include=False,
+ public_deps='talloc',
+ private_library=True
+ )
+
bld.SAMBA_SUBSYSTEM('samba-util-core',
source='''xfile.c data_blob.c util_file.c time.c
signal.c util.c idtree.c fault.c
@@ -119,13 +126,6 @@ if not bld.env.SAMBA_UTIL_CORE_ONLY:
private_library=True
)
- bld.SAMBA_LIBRARY('talloc_report',
- source='talloc_report.c',
- local_include=False,
- public_deps='talloc',
- private_library=True
- )
-
bld.SAMBA_LIBRARY('tevent-util',
source='tevent_unix.c tevent_ntstatus.c tevent_werror.c',
local_include=False,
--
1.9.1
From 17740b0fe716e1302429178097d2355e98a418f8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Mon, 17 Aug 2015 11:55:26 +0200
Subject: [PATCH 4/7] lib: Add the pointer itself to talloc_report_str
A ctdb test found this discrepancy :-)
Signed-off-by: Volker Lendecke <vl at samba.org>
---
lib/util/talloc_report.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/util/talloc_report.c b/lib/util/talloc_report.c
index 8d7d548..9b98347 100644
--- a/lib/util/talloc_report.c
+++ b/lib/util/talloc_report.c
@@ -146,11 +146,11 @@ static void talloc_report_str_helper(const void *ptr, int depth, int max_depth,
state->s = talloc_asprintf_append_largebuf(
state->s, &state->str_len,
- "%*s%-30s contains %6lu bytes in %3lu blocks (ref %d)\n",
+ "%*s%-30s contains %6lu bytes in %3lu blocks (ref %d) %p\n",
depth*4, "", name,
(unsigned long)talloc_total_size(ptr),
(unsigned long)talloc_total_blocks(ptr),
- talloc_reference_count(ptr));
+ talloc_reference_count(ptr), ptr);
}
char *talloc_report_str(TALLOC_CTX *mem_ctx, TALLOC_CTX *root)
--
1.9.1
From 1144443de4bcf28f22c4a748832a5986a5936d21 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 14 Aug 2015 11:40:51 +0200
Subject: [PATCH 5/7] lib: Use dom_sid_equal where appropriate
Signed-off-by: Volker Lendecke <vl at samba.org>
---
libcli/security/util_sid.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index 5127109..7d72d64 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -327,8 +327,9 @@ NTSTATUS add_sid_to_array_unique(TALLOC_CTX *mem_ctx, const struct dom_sid *sid,
uint32_t i;
for (i=0; i<(*num_sids); i++) {
- if (dom_sid_compare(sid, &(*sids)[i]) == 0)
+ if (dom_sid_equal(sid, &(*sids)[i])) {
return NT_STATUS_OK;
+ }
}
return add_sid_to_array(mem_ctx, sid, sids, num_sids);
--
1.9.1
From 1f03acdeece361f108b26c161f29c48f9e686829 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Aug 2015 17:48:41 +0200
Subject: [PATCH 6/7] vfs: Add some {}
The "mode = " from a very casual view looked as if it was part of the
if-condition
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/modules/vfs_default.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 10d0f3b..ac1052e 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -471,8 +471,9 @@ static int vfswrap_mkdir(vfs_handle_struct *handle, const char *path, mode_t mod
if (lp_inherit_acls(SNUM(handle->conn))
&& parent_dirname(talloc_tos(), path, &parent, NULL)
- && (has_dacl = directory_has_default_acl(handle->conn, parent)))
+ && (has_dacl = directory_has_default_acl(handle->conn, parent))) {
mode = (0777 & lp_directory_mask(SNUM(handle->conn)));
+ }
TALLOC_FREE(parent);
--
1.9.1
From 10aeca636fe43a501bab91901d5f77e93f2098fb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 12 Aug 2015 18:32:54 +0200
Subject: [PATCH 7/7] smbd: Remove an unnecessary else branch
"goto out;" is sufficient before
Signed-off-by: Volker Lendecke <vl at samba.org>
---
source3/smbd/dir.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index c700cb7..eebf9b1 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1465,9 +1465,8 @@ bool is_visible_file(connection_struct *conn, const char *dir_path,
if (SMB_VFS_STAT(conn, smb_fname_base) != 0) {
ret = true;
goto out;
- } else {
- *pst = smb_fname_base->st;
}
+ *pst = smb_fname_base->st;
}
/* Honour _hide unreadable_ option */
--
1.9.1
More information about the samba-technical
mailing list