PATCH: Coverity Fixes

Volker Lendecke Volker.Lendecke at SerNet.DE
Mon Nov 18 07:26:07 MST 2013


Hi!

Attached find a few more Coverity fixes. This time some are
not entirely trivial and one touches Kerberos related code,
so I'd appreciate a careful review and push.

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 da3db1c7f1bef265b63c89830f0174c4950589fc Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 16:40:49 +0100
Subject: [PATCH 1/7] libreplace: Fix CID 240518 Resource leak

"d" was left behind after the first call

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

diff --git a/lib/replace/test/os2_delete.c b/lib/replace/test/os2_delete.c
index a11ed3b..17ff46a 100644
--- a/lib/replace/test/os2_delete.c
+++ b/lib/replace/test/os2_delete.c
@@ -103,8 +103,14 @@ int test_readdir_os2_delete(void)
 	create_files();
 
 	d = opendir(TESTDIR "/test0.txt");
-	if (d != NULL) FAILED("opendir() on file succeed");
-	if (errno != ENOTDIR) FAILED("opendir() on file didn't give ENOTDIR");
+	if (d != NULL) {
+		FAILED("opendir() on file succeed");
+		closedir(d);
+	} else {
+		if (errno != ENOTDIR) {
+			FAILED("opendir() on file didn't give ENOTDIR");
+		}
+	}
 
 	d = opendir(TESTDIR);
 
-- 
1.7.9.5


From 267568503e059fe174180fecf243b5882915c703 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 16:55:43 +0100
Subject: [PATCH 2/7] gensec: Rearrange error handling in
 gensec_krb5_session_key

Move error checks before the normal code path. This might fix CID 240726
Resource leak.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/auth/gensec/gensec_krb5.c |   23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/source4/auth/gensec/gensec_krb5.c b/source4/auth/gensec/gensec_krb5.c
index ecc3331..48e5b3b 100644
--- a/source4/auth/gensec/gensec_krb5.c
+++ b/source4/auth/gensec/gensec_krb5.c
@@ -650,19 +650,22 @@ static NTSTATUS gensec_krb5_session_key(struct gensec_security *gensec_security,
 		err = krb5_auth_con_getremotesubkey(context, auth_context, &skey);
 		break;
 	}
-	if (err == 0 && skey != NULL) {
-		DEBUG(10, ("Got KRB5 session key of length %d\n",  
-			   (int)KRB5_KEY_LENGTH(skey)));
-		*session_key = data_blob_talloc(mem_ctx,
-					       KRB5_KEY_DATA(skey), KRB5_KEY_LENGTH(skey));
-		dump_data_pw("KRB5 Session Key:\n", session_key->data, session_key->length);
-
-		krb5_free_keyblock(context, skey);
-		return NT_STATUS_OK;
-	} else {
+	if (err != 0) {
 		DEBUG(10, ("KRB5 error getting session key %d\n", err));
 		return NT_STATUS_NO_USER_SESSION_KEY;
 	}
+	if (skey == NULL) {
+		DEBUG(10, ("%s: No session key returned\n", __func__));
+		return NT_STATUS_NO_USER_SESSION_KEY;
+	}
+	DEBUG(10, ("Got KRB5 session key of length %d\n",
+		   (int)KRB5_KEY_LENGTH(skey)));
+	*session_key = data_blob_talloc(mem_ctx,
+					KRB5_KEY_DATA(skey), KRB5_KEY_LENGTH(skey));
+	dump_data_pw("KRB5 Session Key:\n", session_key->data, session_key->length);
+
+	krb5_free_keyblock(context, skey);
+	return NT_STATUS_OK;
 }
 
 static NTSTATUS gensec_krb5_session_info(struct gensec_security *gensec_security,
-- 
1.7.9.5


From 0e90688af0e1693ccf93920330d2d074b825de00 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 17:21:43 +0100
Subject: [PATCH 3/7] auth: Fix error exit memleaks

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 auth/credentials/credentials_krb5.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/auth/credentials/credentials_krb5.c b/auth/credentials/credentials_krb5.c
index 31fc9d2..b638ac5 100644
--- a/auth/credentials/credentials_krb5.c
+++ b/auth/credentials/credentials_krb5.c
@@ -856,6 +856,7 @@ _PUBLIC_ int cli_credentials_get_server_gss_creds(struct cli_credentials *cred,
 
 	ret = cli_credentials_get_krb5_context(cred, lp_ctx, &smb_krb5_context);
 	if (ret) {
+		talloc_free(mem_ctx);
 		return ret;
 	}
 
@@ -876,6 +877,7 @@ _PUBLIC_ int cli_credentials_get_server_gss_creds(struct cli_credentials *cred,
 	ret = cli_credentials_get_keytab(cred, lp_ctx, &ktc);
 	if (ret) {
 		DEBUG(1, ("Failed to get keytab for GSSAPI server: %s\n", error_message(ret)));
+		talloc_free(mem_ctx);
 		return ret;
 	}
 
-- 
1.7.9.5


From 1e0aa33362e0e536434af05924861e725c3238ba Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 17:34:59 +0100
Subject: [PATCH 4/7] krb: Fix CID 1034833 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/krb5_wrap/keytab_util.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/krb5_wrap/keytab_util.c b/lib/krb5_wrap/keytab_util.c
index f9a8679..398b47f 100644
--- a/lib/krb5_wrap/keytab_util.c
+++ b/lib/krb5_wrap/keytab_util.c
@@ -180,6 +180,7 @@ krb5_error_code kt_copy_one_principal(krb5_context context,
     ret = krb5_kt_resolve (context, from, &src_keytab);
     if (ret) {
 	krb5_set_error_message(context, ret, "resolving src keytab `%s'", from);
+	krb5_free_principal(context, princ);
 	return ret;
     }
 
@@ -187,6 +188,7 @@ krb5_error_code kt_copy_one_principal(krb5_context context,
     if (ret) {
 	krb5_kt_close (context, src_keytab);
 	krb5_set_error_message(context, ret, "resolving dst keytab `%s'", to);
+	krb5_free_principal(context, princ);
 	return ret;
     }
 
@@ -225,6 +227,7 @@ krb5_error_code kt_copy_one_principal(krb5_context context,
 
     krb5_kt_close (context, src_keytab);
     krb5_kt_close (context, dst_keytab);
+    krb5_free_principal(context, princ);
     return ret;
 }
 
-- 
1.7.9.5


From 677518bb06fe814ac0b1f19af6e925338cf71de6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 18:03:17 +0100
Subject: [PATCH 5/7] genrand: Fix CID 710733 Resource leak

"fd" got leaked if we opened it in this routine and got a short read

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/util/genrand.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/lib/util/genrand.c b/lib/util/genrand.c
index 0e5902f..41f253c 100644
--- a/lib/util/genrand.c
+++ b/lib/util/genrand.c
@@ -175,10 +175,17 @@ static int do_reseed(int fd)
 			smb_set_close_on_exec(fd);
 		}
 	}
-	if (fd != -1
-	    && (read(fd, seed_inbuf, sizeof(seed_inbuf)) == sizeof(seed_inbuf))) {
-		seed_random_stream(seed_inbuf, sizeof(seed_inbuf));
-		return fd;
+	if (fd != -1) {
+		if (read(fd, seed_inbuf, sizeof(seed_inbuf)) == sizeof(seed_inbuf)) {
+			seed_random_stream(seed_inbuf, sizeof(seed_inbuf));
+			return fd;
+		}
+		/*
+		 * Short read (can this happen?), close and later re-open
+		 * /dev/urandom
+		 */
+		close(fd);
+		fd = -1;
 	}
 
 	/* Add in some secret file contents */
-- 
1.7.9.5


From 273df97a18731d9d1cd281716eef3c8cc077dc0d Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 18:17:12 +0100
Subject: [PATCH 6/7] pam_winbind: Fix CIDs 1034870,1034871 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 nsswitch/pam_winbind.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/nsswitch/pam_winbind.c b/nsswitch/pam_winbind.c
index 9f85556..1722271 100644
--- a/nsswitch/pam_winbind.c
+++ b/nsswitch/pam_winbind.c
@@ -2399,6 +2399,7 @@ static char* winbind_upn_to_username(struct pwb_context *ctx,
 	char *domain = NULL;
 	char *name;
 	char *p;
+	char *ret;
 
 	/* This cannot work when the winbind separator = @ */
 
@@ -2430,7 +2431,12 @@ static char* winbind_upn_to_username(struct pwb_context *ctx,
 		return NULL;
 	}
 
-	return talloc_asprintf(ctx, "%s%c%s", domain, sep, name);
+	ret = talloc_asprintf(ctx, "%s%c%s", domain, sep, name);
+
+	wbcFreeMemory(domain);
+	wbcFreeMemory(name);
+
+	return ret;
 }
 
 static int _pam_delete_cred(pam_handle_t *pamh, int flags,
-- 
1.7.9.5


From e40dddb92ded9d78528fbff105ed7d46c34b9abb Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 17 Nov 2013 18:19:13 +0100
Subject: [PATCH 7/7] pthreadpool: Fix 1035500 Resource leak

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/pthreadpool/tests.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/source3/lib/pthreadpool/tests.c b/source3/lib/pthreadpool/tests.c
index 95d37b6..f317bea 100644
--- a/source3/lib/pthreadpool/tests.c
+++ b/source3/lib/pthreadpool/tests.c
@@ -86,13 +86,15 @@ static int test_jobs(int num_threads, int num_jobs)
 	}
 
 	ret = pthreadpool_destroy(p);
+
+	free(finished);
+
 	if (ret != 0) {
 		fprintf(stderr, "pthreadpool_destroy failed: %s\n",
 			strerror(ret));
 		return -1;
 	}
 
-	free(finished);
 	return 0;
 }
 
-- 
1.7.9.5



More information about the samba-technical mailing list