[PATCH] Coverity fixes

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Mar 9 04:27:16 MDT 2015


On Sun, Mar 08, 2015 at 05:10:48PM -0400, Ira Cooper wrote:
> On Sun, Mar 8, 2015 at 2:40 PM, Volker Lendecke <Volker.Lendecke at sernet.de>
> wrote:
> 
> > On Sat, Mar 07, 2015 at 07:05:32PM -0500, Ira Cooper wrote:
> > > I tried to push this patchset.  It is causing random failures, or I've
> > > gotten lucky on my last two pushes without it, and "unlucky" on these.
> > >
> > > I'll give this some more thought.  But for now: NAK, due to fail.
> >
> > /memdisk on sn-devel has
> >
> > /dev/loop7             38G   15G 22G  41% /memdisk
> >
> > with just one autobuild running. Maybe boot sn-devel? This
> > led to problems in the past. One of my recent completely
> > innocent fixes took 4 attempts to finally get through.
> >
> 
> Strangely, I pushed Richard Sharpe's change last night.. and it went
> through.  So I'm still suspicious.
> 
> I'll leave the reboot work to those actually doing the work.
> 
> In the changes, the only one I can come up with making a difference is the
> ldb one.  Which we'd both hope wouldn't cause issues.

The attached patchset just survived a private autobuild on sn-devel for
me. It contains the NACKed patchset with some additional but unrelated
Coverity fixes on top. I can't really prove the successful autobuild,
I guess you just have to trust me on this.

Comments?

With best regards,

Volker Lendecke

-- 
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 8967a5d16f06e9c32c27d8f9dc9ad17908f2a777 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 6 Mar 2015 11:02:49 +0000
Subject: [PATCH 01/10] registry: Fix CID 241075 Unchecked return value

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/lib/registry/ldb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/lib/registry/ldb.c b/source4/lib/registry/ldb.c
index d1a8963..1dfffdb 100644
--- a/source4/lib/registry/ldb.c
+++ b/source4/lib/registry/ldb.c
@@ -650,7 +650,8 @@ static WERROR ldb_del_value(TALLOC_CTX *mem_ctx, struct hive_key *key,
 		if (ret != LDB_SUCCESS) {
 			return WERR_FOOBAR;
 		}
-		ldb_msg_add_empty(msg, "type", LDB_FLAG_MOD_DELETE, NULL);
+		ret = ldb_msg_add_empty(msg, "type", LDB_FLAG_MOD_DELETE,
+					NULL);
 		if (ret != LDB_SUCCESS) {
 			return WERR_FOOBAR;
 		}
-- 
1.9.1


From 8a6ef8d22e00c1c1ae56565a89bbc10779e3af03 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Mar 2015 10:24:18 +0000
Subject: [PATCH 02/10] registry: Fix CID 240989 Buffer not null terminated

This makes it clearer that we don't really have a string in .hdr

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/lib/registry/patchfile_preg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/lib/registry/patchfile_preg.c b/source4/lib/registry/patchfile_preg.c
index 28b56dd..8017b58 100644
--- a/source4/lib/registry/patchfile_preg.c
+++ b/source4/lib/registry/patchfile_preg.c
@@ -187,7 +187,7 @@ _PUBLIC_ WERROR reg_preg_diff_save(TALLOC_CTX *ctx, const char *filename,
 		data->fd = STDOUT_FILENO;
 	}
 
-	strncpy(preg_header.hdr, "PReg", 4);
+	memcpy(preg_header.hdr, "PReg", sizeof(preg_header.hdr));
 	SIVAL(&preg_header.version, 0, 1);
 	write(data->fd, (uint8_t *)&preg_header, sizeof(preg_header));
 
-- 
1.9.1


From 9616389ebf73eae0fd589f57958e1eee2f5e656e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Mar 2015 10:29:21 +0000
Subject: [PATCH 03/10] ctdb: Fix 1125553 Buffer not null terminated

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 ctdb/common/system_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ctdb/common/system_linux.c b/ctdb/common/system_linux.c
index 97a57ac..fdb8d12 100644
--- a/ctdb/common/system_linux.c
+++ b/ctdb/common/system_linux.c
@@ -100,7 +100,7 @@ int ctdb_sys_send_arp(const ctdb_sock_addr *addr, const char *iface)
 		}
 
 		DEBUG(DEBUG_DEBUG, (__location__ " Created SOCKET FD:%d for sending arp\n", s));
-		strncpy(ifr.ifr_name, iface, sizeof(ifr.ifr_name)-1);
+		strlcpy(ifr.ifr_name, iface, sizeof(ifr.ifr_name));
 		if (ioctl(s, SIOCGIFINDEX, &ifr) < 0) {
 			DEBUG(DEBUG_CRIT,(__location__ " interface '%s' not found\n", iface));
 			close(s);
-- 
1.9.1


From 8f0ff747c43d1d9cb18e28c6033c4d85bb410cb5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Mar 2015 11:24:33 +0000
Subject: [PATCH 04/10] libreplace: Fix CID 1034926 Destination buffer too
 small

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/replace/test/os2_delete.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/replace/test/os2_delete.c b/lib/replace/test/os2_delete.c
index 9e2115b..0816f61 100644
--- a/lib/replace/test/os2_delete.c
+++ b/lib/replace/test/os2_delete.c
@@ -70,7 +70,8 @@ static int os2_delete(DIR *d)
 	     de && i < READDIR_SIZE; 
 	     de=readdir(d), i++) {
 		offsets[i] = telldir(d);
-		strcpy(names[i], de->d_name);
+		/* strlcpy not available here */
+		snprintf(names[i], sizeof(names[i]), "%s", de->d_name);
 	}
 
 	if (i == 0) {
-- 
1.9.1


From ab119f4db985af532bd75a4f1be0455768a78c87 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sat, 7 Mar 2015 11:39:05 +0000
Subject: [PATCH 05/10] smbcontrol: Simplify do_winbind_offline

This saves 128 bytes of .text on x86-64 with -O3. No idea why...

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/utils/smbcontrol.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/source3/utils/smbcontrol.c b/source3/utils/smbcontrol.c
index 9af0f3e..3946c08 100644
--- a/source3/utils/smbcontrol.c
+++ b/source3/utils/smbcontrol.c
@@ -1093,14 +1093,10 @@ static bool do_winbind_offline(struct tevent_context *ev_ctx,
 	   5 times. */
 
 	for (retry = 0; retry < 5; retry++) {
-		TDB_DATA d;
 		uint8 buf[4];
-
-		ZERO_STRUCT(d);
+		TDB_DATA d = { .dptr = buf, .dsize = sizeof(buf) };
 
 		SIVAL(buf, 0, time(NULL));
-		d.dptr = buf;
-		d.dsize = 4;
 
 		tdb_store_bystring(tdb, "WINBINDD_OFFLINE", d, TDB_INSERT);
 
-- 
1.9.1


From 9d74be204353038c212e1404524a82c16d8aa5f5 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 8 Mar 2015 19:06:38 +0000
Subject: [PATCH 06/10] lib: Fix CID 1034838 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/replace/test/testsuite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/replace/test/testsuite.c b/lib/replace/test/testsuite.c
index 52629ec..5f30e2f 100644
--- a/lib/replace/test/testsuite.c
+++ b/lib/replace/test/testsuite.c
@@ -272,6 +272,7 @@ static int test_strndup(void)
 	x = strndup("bla", 10);
 	if (strcmp(x, "bla") != 0) {
 		printf("failure: strndup [\ninvalid\n]\n");
+		free(x);
 		return false;
 	}
 	free(x);
-- 
1.9.1


From 1eb8d2ca365f84a76cb350b4eaf649188aa634eb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 8 Mar 2015 19:10:50 +0000
Subject: [PATCH 07/10] lib: Fix CID 1034839 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/replace/test/testsuite.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/replace/test/testsuite.c b/lib/replace/test/testsuite.c
index 5f30e2f..4c2fc12 100644
--- a/lib/replace/test/testsuite.c
+++ b/lib/replace/test/testsuite.c
@@ -888,6 +888,7 @@ static int test_utime(void)
 		printf("failure: utime [\n"
 		       "fstat (1) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -897,6 +898,7 @@ static int test_utime(void)
 		printf("failure: utime [\n"
 		       "utime(&u) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -904,6 +906,7 @@ static int test_utime(void)
 		printf("failure: utime [\n"
 		       "fstat (2) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -911,6 +914,7 @@ static int test_utime(void)
 		printf("failure: utime [\n"
 		       "utime(NULL) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -918,6 +922,7 @@ static int test_utime(void)
 		printf("failure: utime [\n"
 		       "fstat (3) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -927,6 +932,7 @@ static int test_utime(void)
 		       "%s: %s(%d) %s %s(%d)\n]\n", \
 		       __location__, \
 		       #a, (int)a, #c, #b, (int)b); \
+		close(fd); \
 		return false; \
 	} \
 } while(0)
@@ -946,6 +952,7 @@ static int test_utime(void)
 
 	unlink(TESTFILE);
 	printf("success: utime\n");
+	close(fd);
 	return true;
 }
 
-- 
1.9.1


From 48a9a3b7a9c666d0ede260a586e9a85120fd5c1d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 8 Mar 2015 19:12:11 +0000
Subject: [PATCH 08/10] lib: Fix CID 1034840 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/replace/test/testsuite.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/replace/test/testsuite.c b/lib/replace/test/testsuite.c
index 4c2fc12..017b8ed 100644
--- a/lib/replace/test/testsuite.c
+++ b/lib/replace/test/testsuite.c
@@ -977,6 +977,7 @@ static int test_utimes(void)
 		printf("failure: utimes [\n"
 		       "fstat (1) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -987,6 +988,7 @@ static int test_utimes(void)
 		printf("failure: utimes [\n"
 		       "utimes(tv) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -994,6 +996,7 @@ static int test_utimes(void)
 		printf("failure: utimes [\n"
 		       "fstat (2) failed - %s\n]\n",
 		       strerror(errno));
+		close(fd);
 		return false;
 	}
 
@@ -1003,6 +1006,7 @@ static int test_utimes(void)
 		       "%s: %s(%d) != %s(%d)\n]\n", \
 		       __location__, \
 		       #a, (int)a, #b, (int)b); \
+		close(fd); \
 		return false; \
 	} \
 } while(0)
@@ -1014,6 +1018,7 @@ static int test_utimes(void)
 
 	unlink(TESTFILE);
 	printf("success: utimes\n");
+	close(fd);
 	return true;
 }
 
-- 
1.9.1


From 4d560b8560697773128e3ca9070fb4b41aca7041 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 8 Mar 2015 19:18:21 +0000
Subject: [PATCH 09/10] tdb: Fix CID 1034841 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tdb/test/run-incompatible.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/tdb/test/run-incompatible.c b/lib/tdb/test/run-incompatible.c
index b8e95b5..5f1b586 100644
--- a/lib/tdb/test/run-incompatible.c
+++ b/lib/tdb/test/run-incompatible.c
@@ -28,15 +28,17 @@ static void log_fn(struct tdb_context *tdb, enum tdb_debug_level level, const ch
 static unsigned int hdr_rwlocks(const char *fname)
 {
 	struct tdb_header hdr;
+	ssize_t nread;
 
 	int fd = open(fname, O_RDONLY);
 	if (fd == -1)
 		return -1;
 
-	if (read(fd, &hdr, sizeof(hdr)) != sizeof(hdr))
-		return -1;
-
+	nread = read(fd, &hdr, sizeof(hdr));
 	close(fd);
+	if (nread != sizeof(hdr)) {
+		return -1;
+	}
 	return hdr.rwlocks;
 }
 
-- 
1.9.1


From d815fe85173da360520cba65da14d977d650f0e7 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 8 Mar 2015 19:21:23 +0000
Subject: [PATCH 10/10] tdb: Fix CID 1034842 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/tdb/test/run-open-during-transaction.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/tdb/test/run-open-during-transaction.c b/lib/tdb/test/run-open-during-transaction.c
index 1605376..9a6c6c1 100644
--- a/lib/tdb/test/run-open-during-transaction.c
+++ b/lib/tdb/test/run-open-during-transaction.c
@@ -74,6 +74,7 @@ static void check_file_intact(int fd)
 	if (pread(fd, contents, st.st_size, 0) != st.st_size) {
 		diag("Read fail");
 		errors++;
+		free(contents);
 		return;
 	}
 
-- 
1.9.1



More information about the samba-technical mailing list