[PATCH] Fix resource leaks and pointer issues

Andreas Schneider asn at samba.org
Wed Oct 25 18:45:02 UTC 2017


Hi,

I have several additional patches which should also be in the next 4.7 
release. Should I just open one bug for all of them or individual ones?

Thanks,


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From eac934c8424de8d52d31db2d0ec71fe667d720c8 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:22:34 +0200
Subject: [PATCH 1/6] libsmbclient: Use const for the user

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/include/libsmbclient.h | 2 +-
 source3/libsmb/libsmb_setget.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/include/libsmbclient.h b/source3/include/libsmbclient.h
index cf67b1d47a4..b41a2924abd 100644
--- a/source3/include/libsmbclient.h
+++ b/source3/include/libsmbclient.h
@@ -491,7 +491,7 @@ smbc_getUser(SMBCCTX *c);
 
 /** Set the username used for making connections */
 void
-smbc_setUser(SMBCCTX *c, char * user);
+smbc_setUser(SMBCCTX *c, const char *user);
 
 /**
  * Get the timeout used for waiting on connections and response data
diff --git a/source3/libsmb/libsmb_setget.c b/source3/libsmb/libsmb_setget.c
index 80ac6739fe8..591cb39c28f 100644
--- a/source3/libsmb/libsmb_setget.c
+++ b/source3/libsmb/libsmb_setget.c
@@ -71,7 +71,7 @@ smbc_getUser(SMBCCTX *c)
 
 /** Set the username used for making connections */
 void
-smbc_setUser(SMBCCTX *c, char * user)
+smbc_setUser(SMBCCTX *c, const char *user)
 {
 	SAFE_FREE(c->user);
 	if (user) {
-- 
2.14.2


>From 9e49d106a186b6026c654a0280887fb188fcf4b9 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:23:02 +0200
Subject: [PATCH 2/6] s4:torture: Avoid useless strdup in libsmbclient test

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source4/torture/libsmbclient/libsmbclient.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/torture/libsmbclient/libsmbclient.c b/source4/torture/libsmbclient/libsmbclient.c
index f6cd8102c4d..16ad35182cd 100644
--- a/source4/torture/libsmbclient/libsmbclient.c
+++ b/source4/torture/libsmbclient/libsmbclient.c
@@ -38,8 +38,8 @@ bool torture_libsmbclient_init_context(struct torture_context *tctx,
 
 	/* yes, libsmbclient API frees the username when freeing the context, so
 	 * have to pass malloced data here */
-	smbc_setUser(ctx, strdup(cli_credentials_get_username(
-			popt_get_cmdline_credentials())));
+	smbc_setUser(ctx,
+		     cli_credentials_get_username(popt_get_cmdline_credentials()));
 
 	*ctx_p = ctx;
 
-- 
2.14.2


>From 9ea3684cc58a89ed663b64c2793aa76444e7f836 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:25:20 +0200
Subject: [PATCH 3/6] s4:pyparam: Fix resource leaks on error

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source4/param/pyparam.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source4/param/pyparam.c b/source4/param/pyparam.c
index 713e608cf1b..80cb38dd347 100644
--- a/source4/param/pyparam.c
+++ b/source4/param/pyparam.c
@@ -331,6 +331,7 @@ static PyObject *py_lp_dump_a_parameter(PyObject *self, PyObject *args)
 
 	if (!ret) {
 		PyErr_Format(PyExc_RuntimeError, "Parameter %s unknown for section %s", param_name, section_name);
+		fclose(f);
 		return NULL;
 	}
 
@@ -479,6 +480,7 @@ static PyObject *py_lp_service_dump(PyObject *self, PyObject *args)
 
 	if (!PyObject_TypeCheck(py_default_service, &PyLoadparmService)) {
 		PyErr_SetNone(PyExc_TypeError);
+		fclose(f);
 		return NULL;
 	}
 
-- 
2.14.2


>From 50631529cd9b77384c7aa727912c07268325ed21 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:30:28 +0200
Subject: [PATCH 4/6] s3:secrets: Do not leak memory of pw and old_pw

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/passdb/machine_account_secrets.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
index 5a0f7a8405c..fbc87c5619c 100644
--- a/source3/passdb/machine_account_secrets.c
+++ b/source3/passdb/machine_account_secrets.c
@@ -1367,6 +1367,8 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 		DBG_ERR("secrets_fetch_domain_sid(%s) failed\n",
 			domain);
 		dbwrap_transaction_cancel(db);
+		SAFE_FREE(old_pw);
+		SAFE_FREE(pw);
 		TALLOC_FREE(frame);
 		return NT_STATUS_CANT_ACCESS_DOMAIN_INFO;
 	}
@@ -1381,6 +1383,8 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 	if (info->account_name == NULL) {
 		DBG_ERR("talloc_asprintf(%s$) failed\n", info->computer_name);
 		dbwrap_transaction_cancel(db);
+		SAFE_FREE(old_pw);
+		SAFE_FREE(pw);
 		TALLOC_FREE(frame);
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1418,6 +1422,8 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 			DBG_ERR("talloc_asprintf(%s#%02X) failed\n",
 				domain, NBT_NAME_PDC);
 			dbwrap_transaction_cancel(db);
+			SAFE_FREE(pw);
+			SAFE_FREE(old_pw);
 			TALLOC_FREE(frame);
 			return NT_STATUS_NO_MEMORY;
 		}
@@ -1438,6 +1444,8 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 		p = kerberos_secrets_fetch_salt_princ();
 		if (p == NULL) {
 			dbwrap_transaction_cancel(db);
+			SAFE_FREE(old_pw);
+			SAFE_FREE(pw);
 			TALLOC_FREE(frame);
 			return NT_STATUS_INTERNAL_ERROR;
 		}
@@ -1445,6 +1453,8 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 		SAFE_FREE(p);
 		if (info->salt_principal == NULL) {
 			dbwrap_transaction_cancel(db);
+			SAFE_FREE(pw);
+			SAFE_FREE(old_pw);
 			TALLOC_FREE(frame);
 			return NT_STATUS_NO_MEMORY;
 		}
@@ -1459,6 +1469,7 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 						     info->salt_principal,
 						     last_set_nt, server,
 						     &info->password);
+	SAFE_FREE(pw);
 	if (!NT_STATUS_IS_OK(status)) {
 		DBG_ERR("secrets_domain_info_password_create(pw) failed "
 			"for %s - %s\n", domain, nt_errstr(status));
@@ -1476,6 +1487,7 @@ NTSTATUS secrets_fetch_or_upgrade_domain_info(const char *domain,
 							     info->salt_principal,
 							     0, server,
 							     &info->old_password);
+		SAFE_FREE(old_pw);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("secrets_domain_info_password_create(old) failed "
 				"for %s - %s\n", domain, nt_errstr(status));
-- 
2.14.2


>From b881e717d1fc16e84337d04af10d681caa64ddf2 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:50:57 +0200
Subject: [PATCH 5/6] s3:passdb: Make sure the salt is fully initialized before
 passing

Otherwise the magic member is not initialized.

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/passdb/machine_account_secrets.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c
index fbc87c5619c..75f31cb04e2 100644
--- a/source3/passdb/machine_account_secrets.c
+++ b/source3/passdb/machine_account_secrets.c
@@ -1090,8 +1090,10 @@ static int secrets_domain_info_kerberos_keys(struct secrets_domain_info1_passwor
 		return krb5_ret;
 	}
 
-	salt.data = discard_const(salt_data);
-	salt.length = strlen(salt_data);
+	salt = (krb5_data) {
+		.data = discard_const(salt_data),
+		.length = strlen(salt_data),
+	};
 
 	ok = convert_string_talloc(keys, CH_UTF16MUNGED, CH_UTF8,
 				   p->cleartext_blob.data,
-- 
2.14.2


>From 15f1d616f036b933c4a1dc02393556dee8bea0e7 Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn at samba.org>
Date: Wed, 25 Oct 2017 19:55:32 +0200
Subject: [PATCH 6/6] s3:modules: Check correct variable for NULL in
 posixacl_xattr

Signed-off-by: Andreas Schneider <asn at samba.org>
---
 source3/modules/posixacl_xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/posixacl_xattr.c b/source3/modules/posixacl_xattr.c
index 759d372facb..8f6f365bff9 100644
--- a/source3/modules/posixacl_xattr.c
+++ b/source3/modules/posixacl_xattr.c
@@ -384,7 +384,7 @@ SMB_ACL_T posixacl_xattr_acl_get_file(vfs_handle_struct *handle,
 		TALLOC_CTX *frame = talloc_stackframe();
 		struct smb_filename *smb_fname_tmp =
 			cp_smb_filename_nostream(frame, smb_fname);
-		if (smb_fname == NULL) {
+		if (smb_fname_tmp == NULL) {
 			errno = ENOMEM;
 			ret = -1;
 		} else {
-- 
2.14.2



More information about the samba-technical mailing list