The branch, master has been updated
       via  3f84a6df454 s3: smbd: Fix fsp/fd leak when looking up a 
non-existent stream name on a file.
       via  c54bec26ad2 s3: tests: Add new test_stream_dir_rename.sh test.
       via  5a3db5105bd s3: provision: Add new streams_xattr_nostrict share - 
needs "strict rename = no".
      from  e3cfb99d286 net: add hint which options can be used with net ads 
dns register command

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 3f84a6df4546e0f1e62dfbcd0b823ea29499a787
Author: Jeremy Allison <j...@samba.org>
Date:   Tue Feb 28 11:20:12 2023 -0800

    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 <j...@samba.org>
    Reviewed-by: Ralph Boehme <s...@samba.org>
    
    Autobuild-User(master): Ralph Böhme <s...@samba.org>
    Autobuild-Date(master): Fri Mar  3 16:37:27 UTC 2023 on atb-devel-224

commit c54bec26ad23b0121b2ddfbf04bc81050f27e6e1
Author: Jeremy Allison <j...@samba.org>
Date:   Tue Feb 28 11:18:10 2023 -0800

    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 <j...@samba.org>
    Reviewed-by: Ralph Boehme <s...@samba.org>

commit 5a3db5105bd8360b245cd35810002740ccff605c
Author: Jeremy Allison <j...@samba.org>
Date:   Tue Feb 28 11:14:34 2023 -0800

    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 <j...@samba.org>
    Reviewed-by: Ralph Boehme <s...@samba.org>

-----------------------------------------------------------------------

Summary of changes:
 selftest/target/Samba3.pm                      |  5 ++
 source3/script/tests/test_stream_dir_rename.sh | 72 ++++++++++++++++++++++++++
 source3/selftest/tests.py                      |  4 ++
 source3/smbd/filename.c                        | 21 ++++++++
 4 files changed, 102 insertions(+)
 create mode 100755 source3/script/tests/test_stream_dir_rename.sh


Changeset truncated at 500 lines:

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
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 <<EOF
+Usage: test_stream_dir_rename.sh SERVER USERNAME PASSWORD PREFIX SMBCLIENT
+EOF
+        exit 1
+fi
+
+SERVER="${1}"
+USERNAME="${2}"
+PASSWORD="${3}"
+PREFIX="${4}"
+SMBCLIENT="${5}"
+SMBCLIENT="$VALGRIND ${SMBCLIENT}"
+shift 5
+
+incdir=$(dirname $0)/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+failed=0
+
+# Do not let deprecated option warnings muck this up
+SAMBA_DEPRECATED_SUPPRESS=1
+export SAMBA_DEPRECATED_SUPPRESS
+
+test_stream_xattr_rename()
+{
+       tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+       #
+       # Test against streams_xattr_nostrict
+       #
+       cat >$tmpfile <<EOF
+deltree stream_xattr_test
+deltree stream_xattr_test1
+mkdir stream_xattr_test
+put ${PREFIX}/smbclient_interactive_prompt_commands stream_xattr_test/file.txt
+get stream_xattr_test/file.txt:abcf
+rename stream_xattr_test stream_xattr_test1
+deltree stream_xattr_test
+deltree stream_xattr_test1
+quit
+EOF
+       cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD 
//$SERVER/streams_xattr_nostrict < $tmpfile 2>&1'
+       eval echo "$cmd"
+       out=$(eval $cmd)
+       ret=$?
+       rm -f $tmpfile
+
+       if [ $ret -ne 0 ]; then
+               echo "$out"
+               echo "failed rename on xattr stream test to test1 with error 
$ret"
+               return 1
+       fi
+
+       echo "$out" | grep "NT_STATUS_ACCESS_DENIED"
+       ret=$?
+       if [ $ret -eq 0 ]; then
+               echo "$out"
+               echo "failed rename on xattr stream with 
NT_STATUS_ACCESS_DENIED"
+               return 1
+       fi
+}
+
+testit "stream_rename" \
+       test_stream_xattr_rename ||
+       failed=$((failed + 1))
+
+testok "$0" "$failed"
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 76b8ad980ee..293882ea69f 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -778,6 +778,10 @@ for env in ["fileserver"]:
                   [os.path.join(samba3srcdir, 
"script/tests/test_veto_files.sh"),
                   '$SERVER', '$SERVER_IP', '$USERNAME', '$PASSWORD', 
'$LOCAL_PATH/veto', smbclient3])
 
+    plantestsuite("samba3.blackbox.stream_dir_rename", env,
+                  [os.path.join(samba3srcdir, 
"script/tests/test_stream_dir_rename.sh"),
+                  '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3])
+
     #
     # tar command tests
     #
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);


-- 
Samba Shared Repository

Reply via email to