[PATCH] smbd crash when change notify is disabled
Ralph Böhme
rb at sernet.de
Sun Aug 23 07:30:34 UTC 2015
On Sat, Aug 15, 2015 at 11:25:47AM +0200, Ralph Böhme wrote:
> On Fri, Aug 14, 2015 at 11:55:11AM +0200, Ralph Böhme wrote:
> > Hi Michael,
> >
> > On Fri, Aug 14, 2015 at 10:14:13AM +0200, Michael Adam wrote:
> > > On 2015-08-12 at 12:53 +0200, Ralph Böhme wrote:
> > > > Hi!
> > > >
> > > > Bug <https://bugzilla.samba.org/show_bug.cgi?id=11444> :
> > > >
> > > > If "change notify = no" is set in smb.conf, notify_ctx is
> > > > NULL. Then in file_free() we pass notify_ctx (= NULL) to
> > > > notify_remove() which doesn't check for that and crashes.
> > > >
> > > > Patch attached. Can someone please review and push if ok?
> > >
> > > This looks good to me, except for one thing:
> > >
> > > Could you use torture_assert_foobar macros in the test suite
> > > instead of adding yet another incarnation of CHECK_STATUS?
> > > (torture_assert_ntstatus_equal_goto() and
> > > torture_assert_ntstatus_ok_goto().)
> >
> > RIP CHECK_STATUS. :) Pushed with this change. Thanks!
>
> got the following error from autobuild:
>
> UNEXPECTED(failure): samba3.rpc.samba3.netlogon.netlogon(nt4_dc)
>
> REASON: Exception: Exception: ../source4/torture/rpc/samba3rpc.c:1426:
> Expression `auth2(torture, cli, wks_creds)' failed: auth2 failed
>
> FAILED (1 failures, 0 errors and 0 unexpected successes in 0
> testsuites)
>
> I tried three times, didn't help. Local autobuild works fine. What's
> going on?
>
> Log:
> <https://git.samba.org/slow/samba-autobuild/samba.stdout>
running the patches incrementally in a private autobuild points at the
last patch being the culprit. No idea why.
Attached is an updated patchset that simply modifies an existing env
(simpleserver) instead of adding a new one. This works.
Please review and push if ok. Thanks!
-Ralph
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From f4126cdd34c3650da667465b597deca81e146da2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 11 Aug 2015 16:49:46 +0200
Subject: [PATCH 1/3] notify: check for valid notify_ctx in notify_remove
notify_ctx will be NULL when "change notify = no" is set in smb.conf.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11444
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
source3/smbd/notify_msg.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/source3/smbd/notify_msg.c b/source3/smbd/notify_msg.c
index 8641983..ea067d0 100644
--- a/source3/smbd/notify_msg.c
+++ b/source3/smbd/notify_msg.c
@@ -183,6 +183,11 @@ NTSTATUS notify_remove(struct notify_context *ctx, void *private_data)
struct iovec iov[2];
NTSTATUS status;
+ /* see if change notify is enabled at all */
+ if (ctx == NULL) {
+ return NT_STATUS_NOT_IMPLEMENTED;
+ }
+
for (listel = ctx->list; listel != NULL; listel = listel->next) {
if (listel->private_data == private_data) {
DLIST_REMOVE(ctx->list, listel);
--
2.1.0
From 0b0825e58ad171061059c6fcf9c2b4b7b11f9d18 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 12 Aug 2015 11:35:27 +0200
Subject: [PATCH 2/3] selftest: add change notify = no to simpleserver env
A subsequent patch will use this env in a torture test.
The aren't any existing tests that make use of change notify, so
disabling change notify in this test environment doesn't impact existing
tests.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11444
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/target/Samba3.pm | 1 +
1 file changed, 1 insertion(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 4638b16..de4346e 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -535,6 +535,7 @@ sub setup_simpleserver($$)
my $simpleserver_options = "
lanman auth = yes
vfs objects = xattr_tdb streams_depot
+ change notify = no
[vfs_aio_fork]
path = $prefix_abs/share
--
2.1.0
From cc540d45b863182e8caa30984cac8cc5eea2539d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 12 Aug 2015 11:06:15 +0200
Subject: [PATCH 3/3] selftest: add a check for disabled change notify
Bug: https://bugzilla.samba.org/show_bug.cgi?id=11444
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/selftest/tests.py | 2 +
source4/selftest/tests.py | 4 +-
source4/torture/smb2/notify_disabled.c | 119 +++++++++++++++++++++++++++++++++
source4/torture/smb2/smb2.c | 1 +
source4/torture/smb2/wscript_build | 2 +-
5 files changed, 126 insertions(+), 2 deletions(-)
create mode 100644 source4/torture/smb2/notify_disabled.c
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 58f2190..ecbb368 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -387,6 +387,8 @@ for t in tests:
elif t == "local.nss":
for env in ["nt4_dc:local", "ad_member:local", "nt4_member:local", "ad_dc:local", "ad_dc_ntvfs:local"]:
plansmbtorture4testsuite(t, env, '//$SERVER/tmp -U$USERNAME%$PASSWORD')
+ elif t == "smb2.change_notify_disabled":
+ plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
elif t == "smb2.notify":
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --signing=required')
plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --signing=required')
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 3bc820c..6c72c34 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -235,7 +235,9 @@ for t in nbt_tests:
# Tests against the NTVFS POSIX backend
ntvfsargs = ["--option=torture:sharedelay=100000", "--option=torture:oplocktimeout=3", "--option=torture:writetimeupdatedelay=500000"]
-smb2 = smbtorture4_testsuites("smb2.")
+# smb2.change_notify_disabled must only run against env fileserver-notify-disabled
+smb2 = filter(lambda x: "smb2.change_notify_disabled" not in x, smbtorture4_testsuites("smb2."))
+
#The QFILEINFO-IPC test needs to be on ipc$
raw = filter(lambda x: "raw.qfileinfo.ipc" not in x, smbtorture4_testsuites("raw."))
base = smbtorture4_testsuites("base.")
diff --git a/source4/torture/smb2/notify_disabled.c b/source4/torture/smb2/notify_disabled.c
new file mode 100644
index 0000000..0faeb51
--- /dev/null
+++ b/source4/torture/smb2/notify_disabled.c
@@ -0,0 +1,119 @@
+/*
+ Unix SMB/CIFS implementation.
+
+ SMB2 notify test suite
+
+ Copyright (C) Stefan Metzmacher 2006
+ Copyright (C) Andrew Tridgell 2009
+ Copyright (C) Ralph Boehme 2015
+
+ 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 <http://www.gnu.org/licenses/>.
+*/
+
+#include "includes.h"
+#include "libcli/smb2/smb2.h"
+#include "libcli/smb2/smb2_calls.h"
+#include "../libcli/smb/smbXcli_base.h"
+
+#include "torture/torture.h"
+#include "torture/smb2/proto.h"
+#include "librpc/gen_ndr/ndr_security.h"
+#include "libcli/security/security.h"
+#include "torture/util.h"
+
+#include "system/filesys.h"
+#include "auth/credentials/credentials.h"
+#include "lib/cmdline/popt_common.h"
+#include "librpc/gen_ndr/security.h"
+
+#include "lib/events/events.h"
+
+#include "libcli/raw/libcliraw.h"
+#include "libcli/raw/raw_proto.h"
+#include "libcli/libcli.h"
+
+#define BASEDIR "test_notify_disabled"
+
+/*
+ basic testing of change notify on directories
+*/
+static bool torture_smb2_notify_disabled(struct torture_context *torture,
+ struct smb2_tree *tree1)
+{
+ bool ret = true;
+ NTSTATUS status;
+ union smb_notify notify;
+ union smb_open io;
+ struct smb2_handle h1;
+ struct smb2_request *req;
+
+ torture_comment(torture, "TESTING CHANGE NOTIFY DISABLED\n");
+
+ smb2_deltree(tree1, BASEDIR);
+ smb2_util_rmdir(tree1, BASEDIR);
+ /*
+ get a handle on the directory
+ */
+ ZERO_STRUCT(io.smb2);
+ io.generic.level = RAW_OPEN_SMB2;
+ io.smb2.in.create_flags = 0;
+ io.smb2.in.desired_access = SEC_FILE_ALL;
+ io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+ io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE;
+ io.smb2.in.alloc_size = 0;
+ io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io.smb2.in.security_flags = 0;
+ io.smb2.in.fname = BASEDIR;
+
+ status = smb2_create(tree1, torture, &(io.smb2));
+ torture_assert_ntstatus_equal_goto(torture, status, NT_STATUS_OK,
+ ret, done, "smb2_create");
+ h1 = io.smb2.out.file.handle;
+
+ ZERO_STRUCT(notify.smb2);
+ notify.smb2.level = RAW_NOTIFY_SMB2;
+ notify.smb2.in.buffer_size = 1000;
+ notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
+ notify.smb2.in.file.handle = h1;
+ notify.smb2.in.recursive = true;
+
+ req = smb2_notify_send(tree1, &(notify.smb2));
+ status = smb2_notify_recv(req, torture, &(notify.smb2));
+ torture_assert_ntstatus_equal_goto(torture, status, NT_STATUS_NOT_IMPLEMENTED,
+ ret, done, "smb2_notify_recv");
+
+ status = smb2_util_close(tree1, h1);
+ torture_assert_ntstatus_equal_goto(torture, status, NT_STATUS_OK,
+ ret, done, "smb2_create");
+
+done:
+ smb2_deltree(tree1, BASEDIR);
+ return ret;
+}
+
+/*
+ basic testing of SMB2 change notify
+*/
+struct torture_suite *torture_smb2_notify_disabled_init(void)
+{
+ struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "change_notify_disabled");
+
+ torture_suite_add_1smb2_test(suite, "notfiy_disabled", torture_smb2_notify_disabled);
+ suite->description = talloc_strdup(suite, "SMB2-CHANGE-NOTIFY-DISABLED tests");
+
+ return suite;
+}
diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
index 853c4f6..0124cf1 100644
--- a/source4/torture/smb2/smb2.c
+++ b/source4/torture/smb2/smb2.c
@@ -155,6 +155,7 @@ NTSTATUS torture_smb2_init(void)
torture_suite_add_suite(suite, torture_smb2_create_init());
torture_suite_add_suite(suite, torture_smb2_acls_init());
torture_suite_add_suite(suite, torture_smb2_notify_init());
+ torture_suite_add_suite(suite, torture_smb2_notify_disabled_init());
torture_suite_add_suite(suite, torture_smb2_durable_open_init());
torture_suite_add_suite(suite, torture_smb2_durable_open_disconnect_init());
torture_suite_add_suite(suite, torture_smb2_durable_v2_open_init());
diff --git a/source4/torture/smb2/wscript_build b/source4/torture/smb2/wscript_build
index 3c374bb..1c593ef 100644
--- a/source4/torture/smb2/wscript_build
+++ b/source4/torture/smb2/wscript_build
@@ -4,7 +4,7 @@ bld.SAMBA_MODULE('TORTURE_SMB2',
source='''connect.c scan.c util.c getinfo.c setinfo.c lock.c notify.c
smb2.c durable_open.c durable_v2_open.c oplock.c dir.c lease.c create.c
acls.c read.c compound.c streams.c ioctl.c rename.c
- session.c delete-on-close.c replay.c''',
+ session.c delete-on-close.c replay.c notify_disabled.c''',
subsystem='smbtorture',
deps='LIBCLI_SMB2 POPT_CREDENTIALS torture NDR_IOCTL',
internal_module=True,
--
2.1.0
More information about the samba-technical
mailing list