The branch, master has been updated via c83dad5 s4:torture: Do not leak file descriptor in smb2 oplock test via 4eefd57 s4:torture: Do not leak memory in libsmbclient test via a762386 s3:libsmbclient: cleanup smbc_setWorkgroup() usage via 20c158c s3:libsmbclient: Use const for setting and getting strings from cb9796f ctdb: Check return values of tevent_req_set_endtime()
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit c83dad5b12cecfdd51fd573b62c0f1ed2b58855a Author: Andreas Schneider <a...@samba.org> Date: Wed May 16 11:46:22 2018 +0200 s4:torture: Do not leak file descriptor in smb2 oplock test Found by Coverity. Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: David Disseldorp <dd...@samba.org> Autobuild-User(master): Andreas Schneider <a...@cryptomilk.org> Autobuild-Date(master): Thu May 17 04:03:21 CEST 2018 on sn-devel-144 commit 4eefd573d24136da37368debb366772c6dbf955f Author: Andreas Schneider <a...@samba.org> Date: Wed May 16 11:44:00 2018 +0200 s4:torture: Do not leak memory in libsmbclient test Found by Coverity. Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: David Disseldorp <dd...@samba.org> commit a762386ccffaace486d8422656b29a4965e13172 Author: David Disseldorp <dd...@samba.org> Date: Wed May 16 16:50:55 2018 +0200 s3:libsmbclient: cleanup smbc_setWorkgroup() usage It now takes a const char *. There's no need to use heap memory here. Signed-off-by: David Disseldorp <dd...@samba.org> Reviewed-by: Andreas Schneider <a...@samba.org> commit 20c158c1f4c35d5176551aa24007d71fa8d1c84e Author: Andreas Schneider <a...@samba.org> Date: Wed May 16 14:49:55 2018 +0200 s3:libsmbclient: Use const for setting and getting strings Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: David Disseldorp <dd...@samba.org> ----------------------------------------------------------------------- Summary of changes: source3/include/libsmbclient.h | 10 +- .../{smbclient-0.3.3.sigs => smbclient-0.4.0.sigs} | 10 +- source3/libsmb/libsmb_context.c | 16 +- source3/libsmb/libsmb_setget.c | 10 +- source3/libsmb/wscript | 2 +- source4/torture/libsmbclient/libsmbclient.c | 205 +++++++++++++++++---- source4/torture/smb2/oplock.c | 7 + 7 files changed, 192 insertions(+), 68 deletions(-) copy source3/libsmb/ABI/{smbclient-0.3.3.sigs => smbclient-0.4.0.sigs} (97%) Changeset truncated at 500 lines: diff --git a/source3/include/libsmbclient.h b/source3/include/libsmbclient.h index 426ed3c..ca5c7f8 100644 --- a/source3/include/libsmbclient.h +++ b/source3/include/libsmbclient.h @@ -537,22 +537,22 @@ int smbc_setConfiguration(SMBCCTX *c, const char *file); /** Get the netbios name used for making connections */ -char * +const char * smbc_getNetbiosName(SMBCCTX *c); /** Set the netbios name used for making connections */ void -smbc_setNetbiosName(SMBCCTX *c, char * netbios_name); +smbc_setNetbiosName(SMBCCTX *c, const char *netbios_name); /** Get the workgroup used for making connections */ -char * +const char * smbc_getWorkgroup(SMBCCTX *c); /** Set the workgroup used for making connections */ -void smbc_setWorkgroup(SMBCCTX *c, char * workgroup); +void smbc_setWorkgroup(SMBCCTX *c, const char *workgroup); /** Get the username used for making connections */ -char * +const char * smbc_getUser(SMBCCTX *c); /** Set the username used for making connections */ diff --git a/source3/libsmb/ABI/smbclient-0.3.3.sigs b/source3/libsmb/ABI/smbclient-0.4.0.sigs similarity index 97% copy from source3/libsmb/ABI/smbclient-0.3.3.sigs copy to source3/libsmb/ABI/smbclient-0.4.0.sigs index 833d0df..170cb52 100644 --- a/source3/libsmb/ABI/smbclient-0.3.3.sigs +++ b/source3/libsmb/ABI/smbclient-0.4.0.sigs @@ -54,7 +54,7 @@ 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_getNetbiosName: const char *(SMBCCTX *) smbc_getOptionBrowseMaxLmbCount: int (SMBCCTX *) smbc_getOptionCaseSensitive: smbc_bool (SMBCCTX *) smbc_getOptionDebugToStderr: smbc_bool (SMBCCTX *) @@ -72,8 +72,8 @@ 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_getUser: const char *(SMBCCTX *) +smbc_getWorkgroup: const 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) @@ -147,7 +147,7 @@ smbc_setFunctionUnlinkPrintJob: void (SMBCCTX *, smbc_unlink_print_job_fn) smbc_setFunctionUtimes: void (SMBCCTX *, smbc_utimes_fn) smbc_setFunctionWrite: void (SMBCCTX *, smbc_write_fn) smbc_setLogCallback: void (SMBCCTX *, void *, smbc_debug_callback_fn) -smbc_setNetbiosName: void (SMBCCTX *, char *) +smbc_setNetbiosName: void (SMBCCTX *, const char *) smbc_setOptionBrowseMaxLmbCount: void (SMBCCTX *, int) smbc_setOptionCaseSensitive: void (SMBCCTX *, smbc_bool) smbc_setOptionDebugToStderr: void (SMBCCTX *, smbc_bool) @@ -166,7 +166,7 @@ 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_setWorkgroup: void (SMBCCTX *, const 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 *) diff --git a/source3/libsmb/libsmb_context.c b/source3/libsmb/libsmb_context.c index 932223c..9e7b083 100644 --- a/source3/libsmb/libsmb_context.c +++ b/source3/libsmb/libsmb_context.c @@ -659,24 +659,16 @@ smbc_init_context(SMBCCTX *context) DEBUG(1, ("Using netbios name %s.\n", smbc_getNetbiosName(context))); if (!smbc_getWorkgroup(context)) { - char *workgroup; + const char *workgroup; if (lp_workgroup()) { - workgroup = SMB_STRDUP(lp_workgroup()); - } - else { + workgroup = lp_workgroup(); + } else { /* TODO: Think about a decent default workgroup */ - workgroup = SMB_STRDUP("samba"); - } - - if (!workgroup) { - TALLOC_FREE(frame); - errno = ENOMEM; - return NULL; + workgroup = "samba"; } smbc_setWorkgroup(context, workgroup); - SAFE_FREE(workgroup); if (!smbc_getWorkgroup(context)) { TALLOC_FREE(frame); diff --git a/source3/libsmb/libsmb_setget.c b/source3/libsmb/libsmb_setget.c index 7a17cad..60b822a 100644 --- a/source3/libsmb/libsmb_setget.c +++ b/source3/libsmb/libsmb_setget.c @@ -29,7 +29,7 @@ /** Get the netbios name used for making connections */ -char * +const char * smbc_getNetbiosName(SMBCCTX *c) { return c->netbios_name; @@ -37,7 +37,7 @@ smbc_getNetbiosName(SMBCCTX *c) /** Set the netbios name used for making connections */ void -smbc_setNetbiosName(SMBCCTX *c, char * netbios_name) +smbc_setNetbiosName(SMBCCTX *c, const char *netbios_name) { SAFE_FREE(c->netbios_name); if (netbios_name) { @@ -46,7 +46,7 @@ smbc_setNetbiosName(SMBCCTX *c, char * netbios_name) } /** Get the workgroup used for making connections */ -char * +const char * smbc_getWorkgroup(SMBCCTX *c) { return c->workgroup; @@ -54,7 +54,7 @@ smbc_getWorkgroup(SMBCCTX *c) /** Set the workgroup used for making connections */ void -smbc_setWorkgroup(SMBCCTX *c, char * workgroup) +smbc_setWorkgroup(SMBCCTX *c, const char *workgroup) { SAFE_FREE(c->workgroup); if (workgroup) { @@ -63,7 +63,7 @@ smbc_setWorkgroup(SMBCCTX *c, char * workgroup) } /** Get the username used for making connections */ -char * +const char * smbc_getUser(SMBCCTX *c) { return c->user; diff --git a/source3/libsmb/wscript b/source3/libsmb/wscript index 15575bc..5482aea 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.3.3', + vnum='0.4.0', pc_files='smbclient.pc') diff --git a/source4/torture/libsmbclient/libsmbclient.c b/source4/torture/libsmbclient/libsmbclient.c index 91579f4..e84ce01 100644 --- a/source4/torture/libsmbclient/libsmbclient.c +++ b/source4/torture/libsmbclient/libsmbclient.c @@ -316,68 +316,193 @@ static bool torture_libsmbclient_readdirplus(struct torture_context *tctx) return true; } - -/* note the strdup for string options on smbc_set calls. I think libsmbclient is - * really doing something wrong here: in smbc_free_context libsmbclient just - * calls free() on the string options so it assumes the callers have malloced - * them before setting them via smbc_set calls. */ - -#define TEST_OPTION_INT(option, val) \ - torture_comment(tctx, "Testing smbc_set" #option "\n");\ - smbc_set ##option(ctx, val);\ - torture_comment(tctx, "Testing smbc_get" #option "\n");\ - torture_assert_int_equal(tctx, smbc_get ##option(ctx), val, "failed " #option); - -#define TEST_OPTION_STRING(option, val) \ - torture_comment(tctx, "Testing smbc_set" #option "\n");\ - smbc_set ##option(ctx, strdup(val));\ - torture_comment(tctx, "Testing smbc_get" #option "\n");\ - torture_assert_str_equal(tctx, smbc_get ##option(ctx), val, "failed " #option); - bool torture_libsmbclient_configuration(struct torture_context *tctx) { SMBCCTX *ctx; + bool ok = true; ctx = smbc_new_context(); torture_assert(tctx, ctx, "failed to get new context"); torture_assert(tctx, smbc_init_context(ctx), "failed to init context"); - TEST_OPTION_INT(Debug, DEBUGLEVEL); - TEST_OPTION_STRING(NetbiosName, "torture_netbios"); - TEST_OPTION_STRING(Workgroup, "torture_workgroup"); - TEST_OPTION_STRING(User, "torture_user"); - TEST_OPTION_INT(Timeout, 12345); - + torture_comment(tctx, "Testing smbc_(set|get)Debug\n"); + smbc_setDebug(ctx, DEBUGLEVEL); + torture_assert_int_equal_goto(tctx, + smbc_getDebug(ctx), + DEBUGLEVEL, + ok, + done, + "failed to set DEBUGLEVEL"); + + torture_comment(tctx, "Testing smbc_(set|get)NetbiosName\n"); + smbc_setNetbiosName(ctx, discard_const("torture_netbios")); + torture_assert_str_equal_goto(tctx, + smbc_getNetbiosName(ctx), + "torture_netbios", + ok, + done, + "failed to set NetbiosName"); + + torture_comment(tctx, "Testing smbc_(set|get)Workgroup\n"); + smbc_setWorkgroup(ctx, discard_const("torture_workgroup")); + torture_assert_str_equal_goto(tctx, + smbc_getWorkgroup(ctx), + "torture_workgroup", + ok, + done, + "failed to set Workgroup"); + + torture_comment(tctx, "Testing smbc_(set|get)User\n"); + smbc_setUser(ctx, "torture_user"); + torture_assert_str_equal_goto(tctx, + smbc_getUser(ctx), + "torture_user", + ok, + done, + "failed to set User"); + + torture_comment(tctx, "Testing smbc_(set|get)Timeout\n"); + smbc_setTimeout(ctx, 12345); + torture_assert_int_equal_goto(tctx, + smbc_getTimeout(ctx), + 12345, + ok, + done, + "failed to set Timeout"); + +done: smbc_free_context(ctx, 1); - return true; + return ok; } bool torture_libsmbclient_options(struct torture_context *tctx) { SMBCCTX *ctx; + bool ok = true; ctx = smbc_new_context(); torture_assert(tctx, ctx, "failed to get new context"); torture_assert(tctx, smbc_init_context(ctx), "failed to init context"); - TEST_OPTION_INT(OptionDebugToStderr, true); - TEST_OPTION_INT(OptionFullTimeNames, true); - TEST_OPTION_INT(OptionOpenShareMode, SMBC_SHAREMODE_DENY_ALL); - /* FIXME: OptionUserData */ - TEST_OPTION_INT(OptionSmbEncryptionLevel, SMBC_ENCRYPTLEVEL_REQUEST); - TEST_OPTION_INT(OptionCaseSensitive, false); - TEST_OPTION_INT(OptionBrowseMaxLmbCount, 2); - TEST_OPTION_INT(OptionUrlEncodeReaddirEntries, true); - TEST_OPTION_INT(OptionOneSharePerServer, true); - TEST_OPTION_INT(OptionUseKerberos, false); - TEST_OPTION_INT(OptionFallbackAfterKerberos, false); - TEST_OPTION_INT(OptionNoAutoAnonymousLogin, true); - TEST_OPTION_INT(OptionUseCCache, true); - + torture_comment(tctx, "Testing smbc_(set|get)OptionDebugToStderr\n"); + smbc_setOptionDebugToStderr(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionDebugToStderr(ctx), + ok, + done, + "failed to set OptionDebugToStderr"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionFullTimeNames\n"); + smbc_setOptionFullTimeNames(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionFullTimeNames(ctx), + ok, + done, + "failed to set OptionFullTimeNames"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionOpenShareMode\n"); + smbc_setOptionOpenShareMode(ctx, SMBC_SHAREMODE_DENY_ALL); + torture_assert_int_equal_goto(tctx, + smbc_getOptionOpenShareMode(ctx), + SMBC_SHAREMODE_DENY_ALL, + ok, + done, + "failed to set OptionOpenShareMode"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionUserData\n"); + smbc_setOptionUserData(ctx, (void *)discard_const("torture_user_data")); + torture_assert_str_equal_goto(tctx, + (const char*)smbc_getOptionUserData(ctx), + "torture_user_data", + ok, + done, + "failed to set OptionUserData"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionSmbEncryptionLevel\n"); + smbc_setOptionSmbEncryptionLevel(ctx, SMBC_ENCRYPTLEVEL_REQUEST); + torture_assert_int_equal_goto(tctx, + smbc_getOptionSmbEncryptionLevel(ctx), + SMBC_ENCRYPTLEVEL_REQUEST, + ok, + done, + "failed to set OptionSmbEncryptionLevel"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionCaseSensitive\n"); + smbc_setOptionCaseSensitive(ctx, false); + torture_assert_goto(tctx, + !smbc_getOptionCaseSensitive(ctx), + ok, + done, + "failed to set OptionCaseSensitive"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionBrowseMaxLmbCount\n"); + smbc_setOptionBrowseMaxLmbCount(ctx, 2); + torture_assert_int_equal_goto(tctx, + smbc_getOptionBrowseMaxLmbCount(ctx), + 2, + ok, + done, + "failed to set OptionBrowseMaxLmbCount"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionUrlEncodeReaddirEntries\n"); + smbc_setOptionUrlEncodeReaddirEntries(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionUrlEncodeReaddirEntries(ctx), + ok, + done, + "failed to set OptionUrlEncodeReaddirEntries"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionOneSharePerServer\n"); + smbc_setOptionOneSharePerServer(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionOneSharePerServer(ctx), + ok, + done, + "failed to set OptionOneSharePerServer"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionUseKerberos\n"); + smbc_setOptionUseKerberos(ctx, false); + torture_assert_goto(tctx, + !smbc_getOptionUseKerberos(ctx), + ok, + done, + "failed to set OptionUseKerberos"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionFallbackAfterKerberos\n"); + smbc_setOptionFallbackAfterKerberos(ctx, false); + torture_assert_goto(tctx, + !smbc_getOptionFallbackAfterKerberos(ctx), + ok, + done, + "failed to set OptionFallbackAfterKerberos"); + + torture_comment(tctx, + "Testing smbc_(set|get)OptionNoAutoAnonymousLogin\n"); + smbc_setOptionNoAutoAnonymousLogin(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionNoAutoAnonymousLogin(ctx), + ok, + done, + "failed to set OptionNoAutoAnonymousLogin"); + + torture_comment(tctx, "Testing smbc_(set|get)OptionUseCCache\n"); + smbc_setOptionUseCCache(ctx, true); + torture_assert_goto(tctx, + smbc_getOptionUseCCache(ctx), + ok, + done, + "failed to set OptionUseCCache"); + +done: smbc_free_context(ctx, 1); - return true; + return ok; } NTSTATUS torture_libsmbclient_init(TALLOC_CTX *ctx) diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c index 6e53007..800a4cf 100644 --- a/source4/torture/smb2/oplock.c +++ b/source4/torture/smb2/oplock.c @@ -4873,17 +4873,20 @@ static int do_child_process(int pipefd, const char *name) ret = fcntl(fd, F_SETSIG, RT_SIGNAL_LEASE); if (ret == -1) { + close(fd); return 3; } ret = fcntl(fd, F_SETLEASE, F_WRLCK); if (ret == -1) { + close(fd); return 4; } /* Tell the parent we're ready. */ ret = sys_write(pipefd, &c, 1); if (ret != 1) { + close(fd); return 5; } @@ -4893,14 +4896,17 @@ static int do_child_process(int pipefd, const char *name) /* Wait for RT_SIGNAL_LEASE or SIGALRM. */ ret = sigsuspend(&empty_set); if (ret != -1 || errno != EINTR) { + close(fd); return 6; } if (got_alarm == 1) { + close(fd); return 10; } if (got_break != 1) { + close(fd); return 7; } @@ -4913,6 +4919,7 @@ static int do_child_process(int pipefd, const char *name) /* Remove our lease. */ ret = fcntl(fd, F_SETLEASE, F_UNLCK); if (ret == -1) { + close(fd); return 8; } -- Samba Shared Repository