[PATCH] Fix resource leaks and pointer issues
Jeremy Allison
jra at samba.org
Fri Oct 27 16:52:44 UTC 2017
On Fri, Oct 27, 2017 at 07:49:40AM +0200, Andreas Schneider wrote:
> 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 :-)
Thanks for the update - pushed !
>
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> 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