[PATCH] Fix resource leaks and pointer issues
Jeremy Allison
jra at samba.org
Wed Oct 25 23:08:30 UTC 2017
On Wed, Oct 25, 2017 at 08:45:02PM +0200, Andreas Schneider wrote:
> 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?
I suggest just one bug - label it "fix resource leaks and pointer issues".
I'll review and get help get it back-ported to 4.7.next, 4.6.next.
Jeremy.
>
> Andreas
>
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> 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