From afcb7c865ca917e61d8aa4a9ed752b531d3db615 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 30 Aug 2018 17:27:08 +0200 Subject: [PATCH 1/6] vfs_delay_inject: adding delay to VFS calls Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 44840ba5b32a2ce7959fd3d7c87822b3159416d3) --- source3/modules/vfs_delay_inject.c | 58 ++++++++++++++++++++++++++++++++++++++ source3/modules/wscript_build | 7 +++++ source3/wscript | 1 + 3 files changed, 66 insertions(+) create mode 100644 source3/modules/vfs_delay_inject.c diff --git a/source3/modules/vfs_delay_inject.c b/source3/modules/vfs_delay_inject.c new file mode 100644 index 00000000000..21fea9b10f4 --- /dev/null +++ b/source3/modules/vfs_delay_inject.c @@ -0,0 +1,58 @@ +/* + * Unix SMB/CIFS implementation. + * Samba VFS module for delay injection in VFS calls + * Copyright (C) Ralph Boehme 2018 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see . + */ + +#include "includes.h" +#include "smbd/smbd.h" + +#undef DBGC_CLASS +#define DBGC_CLASS DBGC_VFS + +static void inject_delay(const char *vfs_func, vfs_handle_struct *handle) +{ + int delay; + + delay = lp_parm_int(SNUM(handle->conn), "delay_inject", vfs_func, 0); + if (delay == 0) { + return; + } + + DBG_DEBUG("Injected delay for [%s] of [%d] ms\n", vfs_func, delay); + + smb_msleep(delay); +} + +static int vfs_delay_inject_ntimes(vfs_handle_struct *handle, + const struct smb_filename *smb_fname, + struct smb_file_time *ft) +{ + inject_delay("ntimes", handle); + + return SMB_VFS_NEXT_NTIMES(handle, smb_fname, ft); +} + +static struct vfs_fn_pointers vfs_delay_inject_fns = { + .ntimes_fn = vfs_delay_inject_ntimes, +}; + +static_decl_vfs; +NTSTATUS vfs_delay_inject_init(TALLOC_CTX *ctx) +{ + return smb_register_vfs(SMB_VFS_INTERFACE_VERSION, "delay_inject", + &vfs_delay_inject_fns); +} diff --git a/source3/modules/wscript_build b/source3/modules/wscript_build index 027283bff87..21ec90944f2 100644 --- a/source3/modules/wscript_build +++ b/source3/modules/wscript_build @@ -555,3 +555,10 @@ bld.SAMBA3_MODULE('vfs_error_inject', init_function='', internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_error_inject'), enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_error_inject')) + +bld.SAMBA3_MODULE('vfs_delay_inject', + subsystem='vfs', + source='vfs_delay_inject.c', + init_function='', + internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_delay_inject'), + enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_delay_inject')) diff --git a/source3/wscript b/source3/wscript index 633a3655b21..31bd7333bdd 100644 --- a/source3/wscript +++ b/source3/wscript @@ -1675,6 +1675,7 @@ main() { if Options.options.enable_selftest or Options.options.developer: default_shared_modules.extend(TO_LIST('vfs_fake_acls vfs_nfs4acl_xattr')) default_shared_modules.extend(TO_LIST('vfs_error_inject')) + default_shared_modules.extend(TO_LIST('vfs_delay_inject')) if conf.CONFIG_SET('AD_DC_BUILD_IS_ENABLED'): default_static_modules.extend(TO_LIST('pdb_samba_dsdb auth_samba4 vfs_dfs_samba4')) -- 2.13.6 From 10388d6da27a40f6c6dca4be662e763bec152ef1 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 31 Aug 2018 08:28:46 +0200 Subject: [PATCH 2/6] s4:selftest: reformat smb2_s3only list No change besides reformatting the list to one entry per line. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 3255822f75163cb38e53f634a5c6b03d46bfaff1) --- source4/selftest/tests.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 09cd55b930a..66989df866a 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -310,7 +310,12 @@ nbt_tests = smbtorture4_testsuites("nbt.") ntvfsargs = ["--option=torture:sharedelay=100000", "--option=torture:oplocktimeout=3", "--option=torture:writetimeupdatedelay=500000"] # Filter smb2 tests that should not run against ad_dc_ntvfs -smb2_s3only = ["smb2.change_notify_disabled", "smb2.dosmode", "smb2.credits", "smb2.kernel-oplocks"] +smb2_s3only = [ + "smb2.change_notify_disabled", + "smb2.dosmode", + "smb2.credits", + "smb2.kernel-oplocks", +] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] #The QFILEINFO-IPC test needs to be on ipc$ -- 2.13.6 From d65347d6050eb858b6c424f7ab90bf6cee453186 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 30 Aug 2018 19:15:19 +0200 Subject: [PATCH 3/6] selftest: add a durable handle test with delayed disconnect Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 5508024a861e7c85e6c837552ad142aa1d5e8eca) --- selftest/knownfail.d/samba3.blackbox | 1 + selftest/target/Samba3.pm | 8 ++ .../script/tests/test_durable_handle_reconnect.sh | 21 +++++ source3/selftest/tests.py | 5 +- source4/selftest/tests.py | 1 + source4/torture/smb2/durable_v2_open.c | 95 ++++++++++++++++++++++ source4/torture/smb2/smb2.c | 2 + 7 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 selftest/knownfail.d/samba3.blackbox create mode 100755 source3/script/tests/test_durable_handle_reconnect.sh diff --git a/selftest/knownfail.d/samba3.blackbox b/selftest/knownfail.d/samba3.blackbox new file mode 100644 index 00000000000..b03a1ac09e2 --- /dev/null +++ b/selftest/knownfail.d/samba3.blackbox @@ -0,0 +1 @@ +^samba3.blackbox.durable_v2_delay.durable_v2_delay\(simpleserver:local\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 24d3d7d6dca..fdbf1fa9a9a 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2175,6 +2175,14 @@ sub provision($$$$$$$$$) copy = tmp vfs objects = error_inject include = $libdir/error_inject.conf + +[delay_inject] + copy = tmp + vfs objects = delay_inject + kernel share modes = no + kernel oplocks = no + posix locking = no + include = $libdir/delay_inject.conf "; close(CONF); diff --git a/source3/script/tests/test_durable_handle_reconnect.sh b/source3/script/tests/test_durable_handle_reconnect.sh new file mode 100755 index 00000000000..bca8e2def96 --- /dev/null +++ b/source3/script/tests/test_durable_handle_reconnect.sh @@ -0,0 +1,21 @@ +#!/bin/sh +# +# Test Durable Handle reconnect with injected delay in the disconnect. +# +# Copyright (C) 2018 Ralph Boehme + +. $(dirname $0)/../../../testprogs/blackbox/subunit.sh +failed=0 + +delay_inject_conf=$(dirname $SMB_CONF_PATH)/delay_inject.conf + +echo 'delay_inject:ntimes = 5000' > $delay_inject_conf + +testit "durable_v2_delay" $VALGRIND \ + $BINDIR/smbtorture //$SERVER_IP/delay_inject \ + -U$USERNAME%$PASSWORD smb2.durable-v2-delay || + failed=$(expr $failed + 1) + +rm $delay_inject_conf + +testok $0 $failed diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 0dca66586b3..069213e53c7 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -464,7 +464,7 @@ tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap elif t == "rpc.samr.passwords.validate": plansmbtorture4testsuite(t, "nt4_dc", 'ncacn_ip_tcp:$SERVER_IP[seal] -U$USERNAME%$PASSWORD', 'over ncacn_ip_tcp ') plansmbtorture4testsuite(t, "ad_dc", 'ncacn_ip_tcp:$SERVER_IP[seal] -U$USERNAME%$PASSWORD', 'over ncacn_ip_tcp ') - elif t == "smb2.durable-open" or t == "smb2.durable-v2-open" or t == "smb2.replay": + elif t == "smb2.durable-open" or t == "smb2.durable-v2-open" or t == "smb2.replay" or t == "smb2.durable-v2-delay": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD') elif t == "base.rw1": @@ -627,6 +627,9 @@ plantestsuite("samba3.blackbox.net_tdb", "simpleserver:local", plantestsuite("samba3.blackbox.smbd_error", "simpleserver:local", [ os.path.join(samba3srcdir, "script/tests/test_smbd_error.sh") ]) +plantestsuite("samba3.blackbox.durable_v2_delay", "simpleserver:local", + [os.path.join(samba3srcdir, "script/tests/test_durable_handle_reconnect.sh")]) + plantestsuite("samba3.blackbox.net_cache_samlogon", "ad_member:local", [ os.path.join(samba3srcdir, "script/tests/test_net_cache_samlogon.sh"), '$SERVER', 'tmp', '$DC_USERNAME', '$DC_PASSWORD']) diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py index 66989df866a..d6b6c5dc4e2 100755 --- a/source4/selftest/tests.py +++ b/source4/selftest/tests.py @@ -315,6 +315,7 @@ smb2_s3only = [ "smb2.dosmode", "smb2.credits", "smb2.kernel-oplocks", + "smb2.durable-v2-delay", ] smb2 = [x for x in smbtorture4_testsuites("smb2.") if x not in smb2_s3only] diff --git a/source4/torture/smb2/durable_v2_open.c b/source4/torture/smb2/durable_v2_open.c index 0a928ec8c26..25e6d27465a 100644 --- a/source4/torture/smb2/durable_v2_open.c +++ b/source4/torture/smb2/durable_v2_open.c @@ -2035,3 +2035,98 @@ struct torture_suite *torture_smb2_durable_v2_open_init(TALLOC_CTX *ctx) return suite; } + +/** + * basic test for doing a durable open + * tcp disconnect, reconnect, do a durable reopen (succeeds) + */ +static bool test_durable_v2_reconnect_delay(struct torture_context *tctx, + struct smb2_tree *tree) +{ + NTSTATUS status; + TALLOC_CTX *mem_ctx = talloc_new(tctx); + char fname[256]; + struct smb2_handle _h; + struct smb2_handle *h = NULL; + struct smb2_create io; + struct GUID create_guid = GUID_random(); + struct smbcli_options options; + uint64_t previous_session_id; + uint8_t b = 0; + bool ret = true; + + options = tree->session->transport->options; + previous_session_id = smb2cli_session_current_id(tree->session->smbXcli); + + /* Choose a random name in case the state is left a little funky. */ + snprintf(fname, 256, "durable_v2_reconnect_delay_%s.dat", + generate_random_str(tctx, 8)); + + smb2_util_unlink(tree, fname); + + smb2_oplock_create_share(&io, fname, + smb2_util_share_access(""), + smb2_util_oplock_level("b")); + io.in.durable_open = false; + io.in.durable_open_v2 = true; + io.in.persistent_open = false; + io.in.create_guid = create_guid; + io.in.timeout = 0; + + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + + _h = io.out.file.handle; + h = &_h; + CHECK_VAL(io.out.oplock_level, smb2_util_oplock_level("b")); + CHECK_VAL(io.out.durable_open_v2, true); + + status = smb2_util_write(tree, *h, &b, 0, 1); + CHECK_STATUS(status, NT_STATUS_OK); + + /* disconnect, leaving the durable open */ + TALLOC_FREE(tree); + + if (!torture_smb2_connection_ext(tctx, previous_session_id, + &options, &tree)) { + torture_warning(tctx, "couldn't reconnect, bailing\n"); + ret = false; + goto done; + } + + ZERO_STRUCT(io); + io.in.fname = fname; + io.in.durable_open_v2 = false; + io.in.durable_handle_v2 = h; + io.in.create_guid = create_guid; + h = NULL; + + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + CHECK_VAL(io.out.oplock_level, smb2_util_oplock_level("b")); + _h = io.out.file.handle; + h = &_h; + +done: + if (h != NULL) { + smb2_util_close(tree, *h); + } + + smb2_util_unlink(tree, fname); + + talloc_free(tree); + + talloc_free(mem_ctx); + + return ret; +} + +struct torture_suite *torture_smb2_durable_v2_delay_init(TALLOC_CTX *ctx) +{ + struct torture_suite *suite = + torture_suite_create(ctx, "durable-v2-delay"); + + torture_suite_add_1smb2_test(suite, "durable_v2_reconnect_delay", test_durable_v2_reconnect_delay); + + return suite; +} diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c index 344cf5a40a5..12f7edf8f86 100644 --- a/source4/torture/smb2/smb2.c +++ b/source4/torture/smb2/smb2.c @@ -163,6 +163,8 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx) torture_smb2_durable_open_disconnect_init(suite)); torture_suite_add_suite(suite, torture_smb2_durable_v2_open_init(suite)); + torture_suite_add_suite(suite, + torture_smb2_durable_v2_delay_init(suite)); torture_suite_add_suite(suite, torture_smb2_dir_init(suite)); torture_suite_add_suite(suite, torture_smb2_lease_init(suite)); torture_suite_add_suite(suite, torture_smb2_compound_init(suite)); -- 2.13.6 From e7228885302cb9eb9a08e0250edf4b5329834d3d Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 30 Aug 2018 15:50:02 +0200 Subject: [PATCH 4/6] s3:smbd: reorder tcon global record deletion and closing files of a tcon As such, this doesn't change overall behaviour, but in case we ever add semantics acting on tcon record changes via an API like dbwrap_watch_send(), this will make a difference as it enforces ordering. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit b70b8503faded81b10859131f08486349876d132) --- source3/smbd/smbXsrv_tcon.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/source3/smbd/smbXsrv_tcon.c b/source3/smbd/smbXsrv_tcon.c index 627eeb166d9..ccf0683cc49 100644 --- a/source3/smbd/smbXsrv_tcon.c +++ b/source3/smbd/smbXsrv_tcon.c @@ -904,6 +904,25 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid) table = tcon->table; tcon->table = NULL; + if (tcon->compat) { + bool ok; + + ok = chdir_current_service(tcon->compat); + if (!ok) { + status = NT_STATUS_INTERNAL_ERROR; + DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): " + "chdir_current_service() failed: %s\n", + tcon->global->tcon_global_id, + tcon->global->share_name, + nt_errstr(status))); + tcon->compat = NULL; + return status; + } + + close_cnum(tcon->compat, vuid); + tcon->compat = NULL; + } + tcon->status = NT_STATUS_NETWORK_NAME_DELETED; global_rec = tcon->global->db_rec; @@ -966,25 +985,6 @@ NTSTATUS smbXsrv_tcon_disconnect(struct smbXsrv_tcon *tcon, uint64_t vuid) } tcon->db_rec = NULL; - if (tcon->compat) { - bool ok; - - ok = chdir_current_service(tcon->compat); - if (!ok) { - status = NT_STATUS_INTERNAL_ERROR; - DEBUG(0, ("smbXsrv_tcon_disconnect(0x%08x, '%s'): " - "chdir_current_service() failed: %s\n", - tcon->global->tcon_global_id, - tcon->global->share_name, - nt_errstr(status))); - tcon->compat = NULL; - return status; - } - - close_cnum(tcon->compat, vuid); - tcon->compat = NULL; - } - return error; } -- 2.13.6 From 54282f0a7f8a892aaf30e9ebdbcb07b5e3653739 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 29 Aug 2018 17:19:29 +0200 Subject: [PATCH 5/6] s3:smbd: let session logoff close files and tcons before deleting the session This avoids a race in durable handle reconnects if the reconnect comes in while the old session is still in the tear-down phase. The new session is supposed to rendezvous with and wait for destruction of the old session, which is internally implemented with dbwrap_watch_send() on the old session record. If the old session deletes the session record before calling file_close_user() which marks all file handles as disconnected, the durable handle reconnect in the new session will fail as the records are not yet marked as disconnected which is a prerequisite. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 8f6edcc1645e0ed35eaec914bd0b672500ce986c) --- selftest/knownfail.d/samba3.blackbox | 1 - source3/smbd/smbXsrv_session.c | 46 ++++++++++++++++++------------------ 2 files changed, 23 insertions(+), 24 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.blackbox diff --git a/selftest/knownfail.d/samba3.blackbox b/selftest/knownfail.d/samba3.blackbox deleted file mode 100644 index b03a1ac09e2..00000000000 --- a/selftest/knownfail.d/samba3.blackbox +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.durable_v2_delay.durable_v2_delay\(simpleserver:local\) diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c index 3d8f0be06ef..3bb3c542fe3 100644 --- a/source3/smbd/smbXsrv_session.c +++ b/source3/smbd/smbXsrv_session.c @@ -1663,6 +1663,29 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session) session->client = NULL; session->status = NT_STATUS_USER_SESSION_DELETED; + if (session->compat) { + file_close_user(sconn, session->compat->vuid); + } + + if (session->tcon_table != NULL) { + /* + * Note: We only have a tcon_table for SMB2. + */ + status = smb2srv_tcon_disconnect_all(session); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(0, ("smbXsrv_session_logoff(0x%08x): " + "smb2srv_tcon_disconnect_all() failed: %s\n", + session->global->session_global_id, + nt_errstr(status))); + error = status; + } + } + + if (session->compat) { + invalidate_vuid(sconn, session->compat->vuid); + session->compat = NULL; + } + global_rec = session->global->db_rec; session->global->db_rec = NULL; if (global_rec == NULL) { @@ -1722,29 +1745,6 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session) } session->db_rec = NULL; - if (session->compat) { - file_close_user(sconn, session->compat->vuid); - } - - if (session->tcon_table != NULL) { - /* - * Note: We only have a tcon_table for SMB2. - */ - status = smb2srv_tcon_disconnect_all(session); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(0, ("smbXsrv_session_logoff(0x%08x): " - "smb2srv_tcon_disconnect_all() failed: %s\n", - session->global->session_global_id, - nt_errstr(status))); - error = status; - } - } - - if (session->compat) { - invalidate_vuid(sconn, session->compat->vuid); - session->compat = NULL; - } - return error; } -- 2.13.6 From 755d671c4e041025b1e78010f1225a6ddf6cb9be Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 30 Aug 2018 15:57:33 +0200 Subject: [PATCH 6/6] s3:smbd: add a comment stating that file_close_user() is redundant for SMB2 Bug: https://bugzilla.samba.org/show_bug.cgi?id=13549 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Sep 1 01:26:35 CEST 2018 on sn-devel-144 (cherry picked from commit 5d95f79f604d90c2646225a0f2470f05dd71e19e) --- source3/smbd/smbXsrv_session.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c index 3bb3c542fe3..dc44480ce39 100644 --- a/source3/smbd/smbXsrv_session.c +++ b/source3/smbd/smbXsrv_session.c @@ -1664,6 +1664,12 @@ NTSTATUS smbXsrv_session_logoff(struct smbXsrv_session *session) session->status = NT_STATUS_USER_SESSION_DELETED; if (session->compat) { + /* + * For SMB2 this is a bit redundant as files are also close + * below via smb2srv_tcon_disconnect_all() -> ... -> + * smbXsrv_tcon_disconnect() -> close_cnum() -> + * file_close_conn(). + */ file_close_user(sconn, session->compat->vuid); } -- 2.13.6