[PATCH] smbd crash when change notify is disabled
Michael Adam
obnox at samba.org
Fri Aug 14 08:14:13 UTC 2015
Hi Ralph,
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().)
Thanks - Michael
> From 7002e5726a99400988f8fbdcc1f00dcb0ea3d308 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>
> ---
> 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 74ef1aa3921d2acd1c04b540a3b45973f0ea96ea 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] selfttest: new testenv "fileserver-notify-disabled"
>
> Add a simple fileserver testenv with change notify disabled. A
> subsequent patch will use this env in a torture test.
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=11444
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> selftest/target/Samba.pm | 1 +
> selftest/target/Samba3.pm | 31 +++++++++++++++++++++++++++++++
> source3/selftest/tests.py | 2 ++
> source4/selftest/tests.py | 4 +++-
> 4 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
> index 9937203..85f40f7 100644
> --- a/selftest/target/Samba.pm
> +++ b/selftest/target/Samba.pm
> @@ -178,6 +178,7 @@ sub get_interface($)
> $interfaces{"promotedvdc"} = 33;
> $interfaces{"rfc2307member"} = 34;
> $interfaces{"fileserver"} = 35;
> + $interfaces{"fileserver-notify-disabled"} = 36;
>
> # update lib/socket_wrapper/socket_wrapper.c
> # #define MAX_WRAPPED_INTERFACES 32
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 4638b16..6e5dbb5 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -180,6 +180,8 @@ sub setup_env($$$)
> return $self->setup_simpleserver("$path/simpleserver");
> } elsif ($envname eq "fileserver") {
> return $self->setup_fileserver("$path/fileserver");
> + } elsif ($envname eq "fileserver_notify_disabled") {
> + return $self->setup_fileserver_notify_disabled("$path/fileserver_notify_disabled");
> } elsif ($envname eq "maptoguest") {
> return $self->setup_maptoguest("$path/maptoguest");
> } elsif ($envname eq "ktest") {
> @@ -676,6 +678,35 @@ sub setup_fileserver($$)
> return $vars;
> }
>
> +sub setup_fileserver_notify_disabled($$)
> +{
> + my ($self, $path) = @_;
> +
> + print "PROVISIONING file server with change notify disabled...\n";
> +
> + my $fileserver_notify_disabled_options = "
> + change notify = no
> + ";
> +
> + my $vars = $self->provision($path,
> + "FILESERVER-NOTIFY-DISABLED",
> + "fileserver-notify-disabled",
> + $fileserver_notify_disabled_options,
> + undef,
> + undef,
> + 1);
> +
> + $vars or return undef;
> +
> + if (not $self->check_or_start($vars, "yes", "no", "yes")) {
> + return undef;
> + }
> +
> + $self->{vars}->{fileserver_notify_disabled} = $vars;
> +
> + return $vars;
> +}
> +
> sub setup_ktest($$$)
> {
> my ($self, $prefix) = @_;
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 58f2190..a385441 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, "fileserver_notify_disabled", '//$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.")
> --
> 2.1.0
>
>
> From acf2e027dc46cb00db7d33dc9de6860c187fe033 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>
> ---
> source4/torture/smb2/notify_disabled.c | 125 +++++++++++++++++++++++++++++++++
> source4/torture/smb2/smb2.c | 1 +
> source4/torture/smb2/wscript_build | 2 +-
> 3 files changed, 127 insertions(+), 1 deletion(-)
> create mode 100644 source4/torture/smb2/notify_disabled.c
>
> diff --git a/source4/torture/smb2/notify_disabled.c b/source4/torture/smb2/notify_disabled.c
> new file mode 100644
> index 0000000..8e1f21c
> --- /dev/null
> +++ b/source4/torture/smb2/notify_disabled.c
> @@ -0,0 +1,125 @@
> +/*
> + 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 CHECK_STATUS(status, correct) do { \
> + if (!NT_STATUS_EQUAL(status, correct)) { \
> + torture_result(torture, TORTURE_FAIL, \
> + "(%s) Incorrect status %s - should be %s\n", \
> + __location__, nt_errstr(status), nt_errstr(correct)); \
> + ret = false; \
> + goto done; \
> + }} while (0)
> +
> +#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));
> + CHECK_STATUS(status, NT_STATUS_OK);
> + 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));
> + CHECK_STATUS(status, NT_STATUS_NOT_IMPLEMENTED);
> +
> + status = smb2_util_close(tree1, h1);
> + CHECK_STATUS(status, NT_STATUS_OK);
> +
> +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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150814/833c5190/attachment.sig>
More information about the samba-technical
mailing list