[PATCH] Fix POSIX unlink on a file containing streams.
Jeremy Allison
jra at samba.org
Tue Jul 19 17:36:01 UTC 2016
Bug https://bugzilla.samba.org/show_bug.cgi?id=12021
is really interesting. It causes the server to recurse
(and clients to timeout and drop connections) when
a client issues a POSIX unlink on an existing file
containing Windows streams.
Essentially when deleting we check for streams
within a file, and do 2 operations - on open for
delete we check that no other process has a stream
open incompatibly, and on close we need to enumerate
the streams by name and delete them. When done on
a POSIX pathname we need to treat the internal
stream names as Windows pathnames or we recurse
(and badness happens :-).
Fix is below and includes regression test.
Back-porting this will be interesting but I
think I know how to make it work for 4.4.x
and 4.3.x.
Please review and push if happy !
Cheers,
Jeremy.
-------------- next part --------------
From d3d2694a5a55b620f58df1876d205b4d912b3a15 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 19 Jul 2016 09:21:08 -0700
Subject: [PATCH 1/2] s3: smbd: Fix delete operations enumerating streams
inside a file. This must always be done as a Windows operation.
When using UNIX extensions to delete a file containing streams,
the open for delete and close operations need to enumerate the
contained streams and do CREATE and UNLINK operations on the
stream names. These must always be done as Windows operations
(remove the SMB_FILENAME_POSIX_PATH flag) as the stream names
are Windows paths.
Without this the create operation under the unlink will
recurse and cause the client to time out (or a server crash).
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12021
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/close.c | 3 ++-
source3/smbd/open.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 9d1f1a9..3c4b9b1 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -205,7 +205,8 @@ NTSTATUS delete_all_streams(connection_struct *conn,
smb_fname->base_name,
stream_info[i].name,
NULL,
- smb_fname->flags);
+ (smb_fname->flags &
+ ~SMB_FILENAME_POSIX_PATH));
if (smb_fname_stream == NULL) {
DEBUG(0, ("talloc_aprintf failed\n"));
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index ab46fe0..2ae6f83 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3936,7 +3936,8 @@ static NTSTATUS open_streams_for_delete(connection_struct *conn,
smb_fname->base_name,
stream_info[i].name,
NULL,
- smb_fname->flags);
+ (smb_fname->flags &
+ ~SMB_FILENAME_POSIX_PATH));
if (smb_fname_cp == NULL) {
status = NT_STATUS_NO_MEMORY;
goto fail;
--
2.8.0.rc3.226.g39d4020
From 4fdd757cd020e60d87ed5cbdf07c0be28f2b93f4 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 19 Jul 2016 09:24:38 -0700
Subject: [PATCH 2/2] s3: torture: Regression test case to specify exactly how
UNIX extensions should act on files with streams.
If a stream is open, refuse the unlink. Ensure UNIX unlink
request can remove a file containing streams.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12021
Signed-off-by: Jeremy Allison <jra at samba.org>
---
selftest/skip | 1 +
selftest/target/Samba3.pm | 2 +-
source3/selftest/tests.py | 3 +-
source3/torture/torture.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 150 insertions(+), 2 deletions(-)
diff --git a/selftest/skip b/selftest/skip
index 4ff7274..ba6718a 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -47,6 +47,7 @@
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-ACL # Fails against the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-EA # Fails against the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-OFD-LOCK # Fails against the s4 ntvfs server
+^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-STREAM-DELETE # Fails against the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).PIDHIGH # Fails against the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4 ntvfs server
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 8a49cdb..f5f4c0c 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1590,7 +1590,7 @@ sub provision($$$$$$$$)
force create mode = 0
directory mask = 0777
force directory mode = 0
- vfs objects = xattr_tdb
+ vfs objects = xattr_tdb streams_depot
[aio]
copy = tmp
aio read size = 1
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index e4b185b..7538f12 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -78,7 +78,8 @@ tests = ["RW1", "RW2", "RW3"]
for t in tests:
plantestsuite("samba3.smbtorture_s3.vfs_aio_fork(simpleserver).%s" % t, "simpleserver", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/vfs_aio_fork', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
-posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-OFD-LOCK" ]
+posix_tests = ["POSIX", "POSIX-APPEND", "POSIX-SYMLINK-ACL", "POSIX-SYMLINK-EA", "POSIX-OFD-LOCK",
+ "POSIX-STREAM-DELETE" ]
for t in posix_tests:
plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/posix_share', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 0926690..f9766bb 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -6014,6 +6014,151 @@ static bool run_acl_symlink_test(int dummy)
}
/*
+ Test POSIX can delete a file containing streams.
+ */
+static bool run_posix_stream_delete(int dummy)
+{
+ struct cli_state *cli1 = NULL;
+ struct cli_state *cli2 = NULL;
+ const char *fname = "streamfile";
+ const char *stream_fname = "streamfile:Zone.Identifier:$DATA";
+ uint16_t fnum1 = (uint16_t)-1;
+ bool correct = false;
+ NTSTATUS status;
+ TALLOC_CTX *frame = NULL;
+
+ frame = talloc_stackframe();
+
+ printf("Starting POSIX stream delete test\n");
+
+ if (!torture_open_connection(&cli1, 0) ||
+ !torture_open_connection(&cli2, 1)) {
+ TALLOC_FREE(frame);
+ return false;
+ }
+
+ smbXcli_conn_set_sockopt(cli1->conn, sockops);
+ smbXcli_conn_set_sockopt(cli2->conn, sockops);
+
+ status = torture_setup_unix_extensions(cli2);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto out;
+ }
+
+ cli_setatr(cli1, fname, 0, 0);
+ cli_unlink(cli1, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ /* Create the file. */
+ status = cli_ntcreate(cli1,
+ fname,
+ 0,
+ READ_CONTROL_ACCESS,
+ 0,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
+ FILE_CREATE,
+ 0x0,
+ 0x0,
+ &fnum1,
+ NULL);
+
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_ntcreate of %s failed (%s)\n",
+ fname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ status = cli_close(cli1, fnum1);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_close of %s failed (%s)\n",
+ fname,
+ nt_errstr(status));
+ goto out;
+ }
+ fnum1 = (uint16_t)-1;
+
+ /* Now create the stream. */
+ status = cli_ntcreate(cli1,
+ stream_fname,
+ 0,
+ FILE_WRITE_DATA,
+ 0,
+ FILE_SHARE_READ|FILE_SHARE_WRITE,
+ FILE_CREATE,
+ 0x0,
+ 0x0,
+ &fnum1,
+ NULL);
+
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_ntcreate of %s failed (%s)\n",
+ stream_fname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ /* Leave the stream handle open... */
+
+ /* POSIX unlink should fail. */
+ status = cli_posix_unlink(cli2, fname);
+ if (NT_STATUS_IS_OK(status)) {
+ printf("cli_posix_unlink of %s succeeded, should have failed\n",
+ fname);
+ goto out;
+ }
+
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_SHARING_VIOLATION)) {
+ printf("cli_posix_unlink of %s failed with (%s) "
+ "should have been NT_STATUS_SHARING_VIOLATION\n",
+ fname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ /* Close the stream handle. */
+ status = cli_close(cli1, fnum1);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_close of %s failed (%s)\n",
+ stream_fname,
+ nt_errstr(status));
+ goto out;
+ }
+ fnum1 = (uint16_t)-1;
+
+ /* POSIX unlink after stream handle closed should succeed. */
+ status = cli_posix_unlink(cli2, fname);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_posix_unlink of %s failed (%s)\n",
+ fname,
+ nt_errstr(status));
+ goto out;
+ }
+
+ printf("POSIX stream delete test passed\n");
+ correct = true;
+
+ out:
+
+ if (fnum1 != (uint16_t)-1) {
+ cli_close(cli1, fnum1);
+ fnum1 = (uint16_t)-1;
+ }
+
+ cli_setatr(cli1, fname, 0, 0);
+ cli_unlink(cli1, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ if (!torture_close_connection(cli1)) {
+ correct = false;
+ }
+ if (!torture_close_connection(cli2)) {
+ correct = false;
+ }
+
+ TALLOC_FREE(frame);
+ return correct;
+}
+
+/*
Test setting EA's are rejected on symlinks.
*/
static bool run_ea_symlink_test(int dummy)
@@ -10297,6 +10442,7 @@ static struct {
{"POSIX-APPEND", run_posix_append, 0},
{"POSIX-SYMLINK-ACL", run_acl_symlink_test, 0},
{"POSIX-SYMLINK-EA", run_ea_symlink_test, 0},
+ {"POSIX-STREAM-DELETE", run_posix_stream_delete, 0},
{"POSIX-OFD-LOCK", run_posix_ofd_lock_test, 0},
{"CASE-INSENSITIVE-CREATE", run_case_insensitive_create, 0},
{"ASYNC-ECHO", run_async_echo, 0},
--
2.8.0.rc3.226.g39d4020
More information about the samba-technical
mailing list