[PATCH] Fix resource leaks and pointer issues

Andreas Schneider asn at samba.org
Fri Oct 27 05:49:40 UTC 2017


On Friday, 27 October 2017 07:14:33 CEST Andreas Schneider via samba-technical 
wrote:
> On Friday, 27 October 2017 01:04:42 CEST Jeremy Allison wrote:
> > On Thu, Oct 26, 2017 at 01:28:42PM -0700, Jeremy Allison via
> > samba-technical
> wrote:
> > > On Thu, Oct 26, 2017 at 07:52:58AM +0200, Andreas Schneider via samba-
> 
> technical wrote:
> > > > On Thursday, 26 October 2017 01:08:30 CEST Jeremy Allison via
> > > > samba-technical> >
> > > > 
> > > > wrote:
> > > > > 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".
> > > > 
> > > > Done, the bug is:
> > > > 
> > > > https://bugzilla.samba.org/show_bug.cgi?id=13101
> > > > 
> > > > and I've added the bug url to all patches attached.
> > > 
> > > Just one note. The change in smbc_getUser() from char *
> > > to const char * is an external library change, so we'll
> > > need to update the shared library version number here.
> > > 
> > > We might want to leave that change out of the back-ports
> > > to 4.7.x and 4.6.x to ensure no ABI change.
> > 
> > There's also an issue with the 3rd patch, f needs to be
> > checked for != stdout before closing.
> > 
> > I pushed the last 3 patches, but you need to re-review
> > these first 3 before push.
> 
> +1 and pushed to autobuild. I'm fine with not backporting the smbclient
> change. The memleak is in the test so it isn't really important for a
> release branch.

The patch is incomplete the signature file is not included, see attached :-)


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
>From 28e5fd5cc52014f9742394f8c00701a59c6b177c 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/3] libsmbclient: Use const for the user

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13101

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/include/libsmbclient.h          |   2 +-
 source3/libsmb/ABI/smbclient-0.3.0.sigs | 179 ++++++++++++++++++++++++++++++++
 source3/libsmb/libsmb_setget.c          |   2 +-
 source3/libsmb/wscript                  |   2 +-
 4 files changed, 182 insertions(+), 3 deletions(-)
 create mode 100644 source3/libsmb/ABI/smbclient-0.3.0.sigs

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/ABI/smbclient-0.3.0.sigs b/source3/libsmb/ABI/smbclient-0.3.0.sigs
new file mode 100644
index 00000000000..736f32a56b9
--- /dev/null
+++ b/source3/libsmb/ABI/smbclient-0.3.0.sigs
@@ -0,0 +1,179 @@
+smbc_chmod: int (const char *, mode_t)
+smbc_close: int (int)
+smbc_closedir: int (int)
+smbc_creat: int (const char *, mode_t)
+smbc_fgetxattr: int (int, const char *, const void *, size_t)
+smbc_flistxattr: int (int, char *, size_t)
+smbc_free_context: int (SMBCCTX *, int)
+smbc_fremovexattr: int (int, const char *)
+smbc_fsetxattr: int (int, const char *, const void *, size_t, int)
+smbc_fstat: int (int, struct stat *)
+smbc_fstatvfs: int (int, struct statvfs *)
+smbc_ftruncate: int (int, off_t)
+smbc_getDebug: int (SMBCCTX *)
+smbc_getFunctionAddCachedServer: smbc_add_cached_srv_fn (SMBCCTX *)
+smbc_getFunctionAuthData: smbc_get_auth_data_fn (SMBCCTX *)
+smbc_getFunctionAuthDataWithContext: smbc_get_auth_data_with_context_fn (SMBCCTX *)
+smbc_getFunctionCheckServer: smbc_check_server_fn (SMBCCTX *)
+smbc_getFunctionChmod: smbc_chmod_fn (SMBCCTX *)
+smbc_getFunctionClose: smbc_close_fn (SMBCCTX *)
+smbc_getFunctionClosedir: smbc_closedir_fn (SMBCCTX *)
+smbc_getFunctionCreat: smbc_creat_fn (SMBCCTX *)
+smbc_getFunctionFstat: smbc_fstat_fn (SMBCCTX *)
+smbc_getFunctionFstatVFS: smbc_fstatvfs_fn (SMBCCTX *)
+smbc_getFunctionFstatdir: smbc_fstatdir_fn (SMBCCTX *)
+smbc_getFunctionFtruncate: smbc_ftruncate_fn (SMBCCTX *)
+smbc_getFunctionGetCachedServer: smbc_get_cached_srv_fn (SMBCCTX *)
+smbc_getFunctionGetdents: smbc_getdents_fn (SMBCCTX *)
+smbc_getFunctionGetxattr: smbc_getxattr_fn (SMBCCTX *)
+smbc_getFunctionListPrintJobs: smbc_list_print_jobs_fn (SMBCCTX *)
+smbc_getFunctionListxattr: smbc_listxattr_fn (SMBCCTX *)
+smbc_getFunctionLseek: smbc_lseek_fn (SMBCCTX *)
+smbc_getFunctionLseekdir: smbc_lseekdir_fn (SMBCCTX *)
+smbc_getFunctionMkdir: smbc_mkdir_fn (SMBCCTX *)
+smbc_getFunctionNotify: smbc_notify_fn (SMBCCTX *)
+smbc_getFunctionOpen: smbc_open_fn (SMBCCTX *)
+smbc_getFunctionOpenPrintJob: smbc_open_print_job_fn (SMBCCTX *)
+smbc_getFunctionOpendir: smbc_opendir_fn (SMBCCTX *)
+smbc_getFunctionPrintFile: smbc_print_file_fn (SMBCCTX *)
+smbc_getFunctionPurgeCachedServers: smbc_purge_cached_fn (SMBCCTX *)
+smbc_getFunctionRead: smbc_read_fn (SMBCCTX *)
+smbc_getFunctionReaddir: smbc_readdir_fn (SMBCCTX *)
+smbc_getFunctionRemoveCachedServer: smbc_remove_cached_srv_fn (SMBCCTX *)
+smbc_getFunctionRemoveUnusedServer: smbc_remove_unused_server_fn (SMBCCTX *)
+smbc_getFunctionRemovexattr: smbc_removexattr_fn (SMBCCTX *)
+smbc_getFunctionRename: smbc_rename_fn (SMBCCTX *)
+smbc_getFunctionRmdir: smbc_rmdir_fn (SMBCCTX *)
+smbc_getFunctionSetxattr: smbc_setxattr_fn (SMBCCTX *)
+smbc_getFunctionSplice: smbc_splice_fn (SMBCCTX *)
+smbc_getFunctionStat: smbc_stat_fn (SMBCCTX *)
+smbc_getFunctionStatVFS: smbc_statvfs_fn (SMBCCTX *)
+smbc_getFunctionTelldir: smbc_telldir_fn (SMBCCTX *)
+smbc_getFunctionUnlink: smbc_unlink_fn (SMBCCTX *)
+smbc_getFunctionUnlinkPrintJob: smbc_unlink_print_job_fn (SMBCCTX *)
+smbc_getFunctionUtimes: smbc_utimes_fn (SMBCCTX *)
+smbc_getFunctionWrite: smbc_write_fn (SMBCCTX *)
+smbc_getNetbiosName: char *(SMBCCTX *)
+smbc_getOptionBrowseMaxLmbCount: int (SMBCCTX *)
+smbc_getOptionCaseSensitive: smbc_bool (SMBCCTX *)
+smbc_getOptionDebugToStderr: smbc_bool (SMBCCTX *)
+smbc_getOptionFallbackAfterKerberos: smbc_bool (SMBCCTX *)
+smbc_getOptionFullTimeNames: smbc_bool (SMBCCTX *)
+smbc_getOptionNoAutoAnonymousLogin: smbc_bool (SMBCCTX *)
+smbc_getOptionOneSharePerServer: smbc_bool (SMBCCTX *)
+smbc_getOptionOpenShareMode: smbc_share_mode (SMBCCTX *)
+smbc_getOptionSmbEncryptionLevel: smbc_smb_encrypt_level (SMBCCTX *)
+smbc_getOptionUrlEncodeReaddirEntries: smbc_bool (SMBCCTX *)
+smbc_getOptionUseCCache: smbc_bool (SMBCCTX *)
+smbc_getOptionUseKerberos: smbc_bool (SMBCCTX *)
+smbc_getOptionUseNTHash: smbc_bool (SMBCCTX *)
+smbc_getOptionUserData: void *(SMBCCTX *)
+smbc_getPort: uint16_t (SMBCCTX *)
+smbc_getServerCacheData: struct smbc_server_cache *(SMBCCTX *)
+smbc_getTimeout: int (SMBCCTX *)
+smbc_getUser: char *(SMBCCTX *)
+smbc_getWorkgroup: char *(SMBCCTX *)
+smbc_getdents: int (unsigned int, struct smbc_dirent *, int)
+smbc_getxattr: int (const char *, const char *, const void *, size_t)
+smbc_init: int (smbc_get_auth_data_fn, int)
+smbc_init_context: SMBCCTX *(SMBCCTX *)
+smbc_lgetxattr: int (const char *, const char *, const void *, size_t)
+smbc_list_print_jobs: int (const char *, smbc_list_print_job_fn)
+smbc_listxattr: int (const char *, char *, size_t)
+smbc_llistxattr: int (const char *, char *, size_t)
+smbc_lremovexattr: int (const char *, const char *)
+smbc_lseek: off_t (int, off_t, int)
+smbc_lseekdir: int (int, off_t)
+smbc_lsetxattr: int (const char *, const char *, const void *, size_t, int)
+smbc_mkdir: int (const char *, mode_t)
+smbc_new_context: SMBCCTX *(void)
+smbc_notify: int (int, smbc_bool, uint32_t, unsigned int, smbc_notify_callback_fn, void *)
+smbc_open: int (const char *, int, mode_t)
+smbc_open_print_job: int (const char *)
+smbc_opendir: int (const char *)
+smbc_option_get: void *(SMBCCTX *, char *)
+smbc_option_set: void (SMBCCTX *, char *, ...)
+smbc_print_file: int (const char *, const char *)
+smbc_read: ssize_t (int, void *, size_t)
+smbc_readdir: struct smbc_dirent *(unsigned int)
+smbc_removexattr: int (const char *, const char *)
+smbc_rename: int (const char *, const char *)
+smbc_rmdir: int (const char *)
+smbc_setDebug: void (SMBCCTX *, int)
+smbc_setFunctionAddCachedServer: void (SMBCCTX *, smbc_add_cached_srv_fn)
+smbc_setFunctionAuthData: void (SMBCCTX *, smbc_get_auth_data_fn)
+smbc_setFunctionAuthDataWithContext: void (SMBCCTX *, smbc_get_auth_data_with_context_fn)
+smbc_setFunctionCheckServer: void (SMBCCTX *, smbc_check_server_fn)
+smbc_setFunctionChmod: void (SMBCCTX *, smbc_chmod_fn)
+smbc_setFunctionClose: void (SMBCCTX *, smbc_close_fn)
+smbc_setFunctionClosedir: void (SMBCCTX *, smbc_closedir_fn)
+smbc_setFunctionCreat: void (SMBCCTX *, smbc_creat_fn)
+smbc_setFunctionFstat: void (SMBCCTX *, smbc_fstat_fn)
+smbc_setFunctionFstatVFS: void (SMBCCTX *, smbc_fstatvfs_fn)
+smbc_setFunctionFstatdir: void (SMBCCTX *, smbc_fstatdir_fn)
+smbc_setFunctionFtruncate: void (SMBCCTX *, smbc_ftruncate_fn)
+smbc_setFunctionGetCachedServer: void (SMBCCTX *, smbc_get_cached_srv_fn)
+smbc_setFunctionGetdents: void (SMBCCTX *, smbc_getdents_fn)
+smbc_setFunctionGetxattr: void (SMBCCTX *, smbc_getxattr_fn)
+smbc_setFunctionListPrintJobs: void (SMBCCTX *, smbc_list_print_jobs_fn)
+smbc_setFunctionListxattr: void (SMBCCTX *, smbc_listxattr_fn)
+smbc_setFunctionLseek: void (SMBCCTX *, smbc_lseek_fn)
+smbc_setFunctionLseekdir: void (SMBCCTX *, smbc_lseekdir_fn)
+smbc_setFunctionMkdir: void (SMBCCTX *, smbc_mkdir_fn)
+smbc_setFunctionNotify: void (SMBCCTX *, smbc_notify_fn)
+smbc_setFunctionOpen: void (SMBCCTX *, smbc_open_fn)
+smbc_setFunctionOpenPrintJob: void (SMBCCTX *, smbc_open_print_job_fn)
+smbc_setFunctionOpendir: void (SMBCCTX *, smbc_opendir_fn)
+smbc_setFunctionPrintFile: void (SMBCCTX *, smbc_print_file_fn)
+smbc_setFunctionPurgeCachedServers: void (SMBCCTX *, smbc_purge_cached_fn)
+smbc_setFunctionRead: void (SMBCCTX *, smbc_read_fn)
+smbc_setFunctionReaddir: void (SMBCCTX *, smbc_readdir_fn)
+smbc_setFunctionRemoveCachedServer: void (SMBCCTX *, smbc_remove_cached_srv_fn)
+smbc_setFunctionRemoveUnusedServer: void (SMBCCTX *, smbc_remove_unused_server_fn)
+smbc_setFunctionRemovexattr: void (SMBCCTX *, smbc_removexattr_fn)
+smbc_setFunctionRename: void (SMBCCTX *, smbc_rename_fn)
+smbc_setFunctionRmdir: void (SMBCCTX *, smbc_rmdir_fn)
+smbc_setFunctionSetxattr: void (SMBCCTX *, smbc_setxattr_fn)
+smbc_setFunctionSplice: void (SMBCCTX *, smbc_splice_fn)
+smbc_setFunctionStat: void (SMBCCTX *, smbc_stat_fn)
+smbc_setFunctionStatVFS: void (SMBCCTX *, smbc_statvfs_fn)
+smbc_setFunctionTelldir: void (SMBCCTX *, smbc_telldir_fn)
+smbc_setFunctionUnlink: void (SMBCCTX *, smbc_unlink_fn)
+smbc_setFunctionUnlinkPrintJob: void (SMBCCTX *, smbc_unlink_print_job_fn)
+smbc_setFunctionUtimes: void (SMBCCTX *, smbc_utimes_fn)
+smbc_setFunctionWrite: void (SMBCCTX *, smbc_write_fn)
+smbc_setNetbiosName: void (SMBCCTX *, char *)
+smbc_setOptionBrowseMaxLmbCount: void (SMBCCTX *, int)
+smbc_setOptionCaseSensitive: void (SMBCCTX *, smbc_bool)
+smbc_setOptionDebugToStderr: void (SMBCCTX *, smbc_bool)
+smbc_setOptionFallbackAfterKerberos: void (SMBCCTX *, smbc_bool)
+smbc_setOptionFullTimeNames: void (SMBCCTX *, smbc_bool)
+smbc_setOptionNoAutoAnonymousLogin: void (SMBCCTX *, smbc_bool)
+smbc_setOptionOneSharePerServer: void (SMBCCTX *, smbc_bool)
+smbc_setOptionOpenShareMode: void (SMBCCTX *, smbc_share_mode)
+smbc_setOptionSmbEncryptionLevel: void (SMBCCTX *, smbc_smb_encrypt_level)
+smbc_setOptionUrlEncodeReaddirEntries: void (SMBCCTX *, smbc_bool)
+smbc_setOptionUseCCache: void (SMBCCTX *, smbc_bool)
+smbc_setOptionUseKerberos: void (SMBCCTX *, smbc_bool)
+smbc_setOptionUseNTHash: void (SMBCCTX *, smbc_bool)
+smbc_setOptionUserData: void (SMBCCTX *, void *)
+smbc_setPort: void (SMBCCTX *, uint16_t)
+smbc_setServerCacheData: void (SMBCCTX *, struct smbc_server_cache *)
+smbc_setTimeout: void (SMBCCTX *, int)
+smbc_setUser: void (SMBCCTX *, const char *)
+smbc_setWorkgroup: void (SMBCCTX *, char *)
+smbc_set_context: SMBCCTX *(SMBCCTX *)
+smbc_set_credentials: void (const char *, const char *, const char *, smbc_bool, const char *)
+smbc_set_credentials_with_fallback: void (SMBCCTX *, const char *, const char *, const char *)
+smbc_setxattr: int (const char *, const char *, const void *, size_t, int)
+smbc_stat: int (const char *, struct stat *)
+smbc_statvfs: int (char *, struct statvfs *)
+smbc_telldir: off_t (int)
+smbc_unlink: int (const char *)
+smbc_unlink_print_job: int (const char *, int)
+smbc_urldecode: int (char *, char *, size_t)
+smbc_urlencode: int (char *, char *, int)
+smbc_utime: int (const char *, struct utimbuf *)
+smbc_utimes: int (const char *, struct timeval *)
+smbc_version: const char *(void)
+smbc_write: ssize_t (int, const void *, size_t)
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) {
diff --git a/source3/libsmb/wscript b/source3/libsmb/wscript
index 6d862f71998..c6ad6861190 100644
--- a/source3/libsmb/wscript
+++ b/source3/libsmb/wscript
@@ -27,5 +27,5 @@ def build(bld):
                        public_headers='../include/libsmbclient.h',
                        abi_directory='ABI',
                        abi_match='smbc_*',
-                       vnum='0.2.3',
+                       vnum='0.3.0',
                        pc_files='smbclient.pc')
-- 
2.14.2


>From ee688256771e5d0aae7ac90d2b05adb7acebd733 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/3] s4:torture: Avoid useless strdup in libsmbclient test

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13101

Signed-off-by: Andreas Schneider <asn at samba.org>
Reviewed-by: Jeremy Allison <jra 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 4756adf1be8f85da9b402ebc353341094db833e2 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/3] s4:pyparam: Fix resource leaks on error

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13101

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

diff --git a/source4/param/pyparam.c b/source4/param/pyparam.c
index 713e608cf1b..f16c2c0b227 100644
--- a/source4/param/pyparam.c
+++ b/source4/param/pyparam.c
@@ -331,6 +331,9 @@ 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);
+		if (f != stdout) {
+			fclose(f);
+		}
 		return NULL;
 	}
 
@@ -479,6 +482,9 @@ static PyObject *py_lp_service_dump(PyObject *self, PyObject *args)
 
 	if (!PyObject_TypeCheck(py_default_service, &PyLoadparmService)) {
 		PyErr_SetNone(PyExc_TypeError);
+		if (f != stdout) {
+			fclose(f);
+		}
 		return NULL;
 	}
 
-- 
2.14.2



More information about the samba-technical mailing list