From d0afe9adce1b728fc346952ee5dd4cd074401127 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 28 Feb 2023 11:14:34 -0800 Subject: [PATCH 1/3] s3: provision: Add new streams_xattr_nostrict share - needs "strict rename = no". The bug we're testing for needs "strict rename = no" (the default), but the existing streams_xattr share uses "strict rename = yes" from the [global] section. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15314 Signed-off-by: Jeremy Allison --- selftest/target/Samba3.pm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 476f59c8783..15b13f2920f 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -3461,6 +3461,11 @@ sub provision($$) copy = tmp vfs objects = streams_xattr xattr_tdb +[streams_xattr_nostrict] + copy = tmp + strict rename = no + vfs objects = streams_xattr xattr_tdb + [acl_streams_xattr] copy = tmp vfs objects = acl_xattr streams_xattr fake_acls xattr_tdb -- 2.34.1 From 1086ff7c5db90c35f13ac240d0f1debe799e8add Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 28 Feb 2023 11:18:10 -0800 Subject: [PATCH 2/3] s3: tests: Add new test_stream_dir_rename.sh test. Shows we are leaking an fsp/fd if we request a non-existent stream on a file. This then causes rename of a directory containing the file to be denied, as it thinks we have an existing open file below it. Add knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15314 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/stream_rename | 1 + .../script/tests/test_stream_dir_rename.sh | 72 +++++++++++++++++++ source3/selftest/tests.py | 4 ++ 3 files changed, 77 insertions(+) create mode 100644 selftest/knownfail.d/stream_rename create mode 100755 source3/script/tests/test_stream_dir_rename.sh diff --git a/selftest/knownfail.d/stream_rename b/selftest/knownfail.d/stream_rename new file mode 100644 index 00000000000..2dccb826cd6 --- /dev/null +++ b/selftest/knownfail.d/stream_rename @@ -0,0 +1 @@ +^samba3.blackbox.stream_dir_rename.stream_rename\(fileserver\) diff --git a/source3/script/tests/test_stream_dir_rename.sh b/source3/script/tests/test_stream_dir_rename.sh new file mode 100755 index 00000000000..7ac3194f649 --- /dev/null +++ b/source3/script/tests/test_stream_dir_rename.sh @@ -0,0 +1,72 @@ +#!/bin/sh +# +# Test a stream can rename a directory once an invalid stream path below it was requested. +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=15314 + +if [ $# -lt 5 ]; then + cat <$tmpfile < Date: Tue, 28 Feb 2023 11:20:12 -0800 Subject: [PATCH 3/3] s3: smbd: Fix fsp/fd leak when looking up a non-existent stream name on a file. When open_stream_pathref_fsp() returns NT_STATUS_OBJECT_NAME_NOT_FOUND, smb_fname_rel->fsp has been set to NULL, so we must free base_fsp separately to prevent fd-leaks when opening a stream that doesn't exist. Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15314 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/stream_rename | 1 - source3/smbd/filename.c | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/stream_rename diff --git a/selftest/knownfail.d/stream_rename b/selftest/knownfail.d/stream_rename deleted file mode 100644 index 2dccb826cd6..00000000000 --- a/selftest/knownfail.d/stream_rename +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.stream_dir_rename.stream_rename\(fileserver\) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index e9775387d11..78f552de9b2 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -1386,6 +1386,16 @@ static NTSTATUS filename_convert_dirfsp_nosymlink( status = NT_STATUS_NO_MEMORY; goto fail; } + /* + * When open_stream_pathref_fsp() returns + * NT_STATUS_OBJECT_NAME_NOT_FOUND, smb_fname_rel->fsp + * has been set to NULL, so we must free base_fsp separately + * to prevent fd-leaks when opening a stream that doesn't + * exist. + */ + fd_close(base_fsp); + file_free(NULL, base_fsp); + base_fsp = NULL; goto done; } @@ -1402,6 +1412,17 @@ done: return NT_STATUS_OK; fail: + /* + * If open_stream_pathref_fsp() returns an error, smb_fname_rel->fsp + * has been set to NULL, so we must free base_fsp separately + * to prevent fd-leaks when opening a stream that doesn't + * exist. + */ + if (base_fsp != NULL) { + fd_close(base_fsp); + file_free(NULL, base_fsp); + base_fsp = NULL; + } TALLOC_FREE(dirname); TALLOC_FREE(smb_dirname); TALLOC_FREE(smb_fname_rel); -- 2.34.1