The branch, master has been updated via 82beaf8 s3: tests: Regression test to ensure we can never return a DIRECTORY attribute on a stream. via 118e77d s3: smbd. Generic fix for incorrect reporting of stream dos attributes on a directory via 4d839d0 s3: vfs: vfs_streams_xattr: Don't blindly re-use the base file mode bits. from dff196a credentials: Fix CID 1414796 Explicit null dereferenced
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 82beaf868f252c4bc975ddafd80240af6f679b83 Author: Jeremy Allison <j...@samba.org> Date: Wed Apr 11 10:33:22 2018 -0700 s3: tests: Regression test to ensure we can never return a DIRECTORY attribute on a stream. Tests streams_xattr and also streams_depot. Inspired from a real-world test case by Andrew Walker <awal...@ixsystems.com>. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13380 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Böhme <s...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Thu Apr 12 02:04:28 CEST 2018 on sn-devel-144 commit 118e77d86a7171f589f805fa4f63246b0cb63672 Author: Jeremy Allison <j...@samba.org> Date: Wed Apr 11 11:05:14 2018 -0700 s3: smbd. Generic fix for incorrect reporting of stream dos attributes on a directory According to MS-FSA a stream name does not have separate DOS attribute metadata, so we must return the DOS attribute from the base filename. With one caveat, a non-default stream name can never be a directory. As this is common to all streams data stores, we handle it here instead of inside all stream VFS modules. Otherwise identical logic would have to be added to all streams modules in their [f]get_dos_attribute_fn() VFS calls. Found in real-world use case by Andrew Walker <awal...@ixsystems.com>. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13380 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Ralph Böhme <s...@samba.org> commit 4d839d0f46b723ed6809bb932b9ebe4ead2cec82 Author: Jeremy Allison <j...@samba.org> Date: Wed Apr 11 08:41:00 2018 -0700 s3: vfs: vfs_streams_xattr: Don't blindly re-use the base file mode bits. When returning the stat struct for an xattr stream, we originally base the st_ex_mode field on the value from the base file containing the xattr. If the base file is a directory, it will have S_IFDIR set in st_ex_mode, but streams can never be directories, they must be reported as regular files. The original code OR'ed in S_IFREG, but neglected to AND out S_IFDIR. Note this is not a complete to fix bug 13380 as it doesn't fix the generic case with all streams modules. See later fix and regression test. Found in real-world use case by Andrew Walker <awal...@ixsystems.com>. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13380 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Volker Lendecke <v...@samba.org> Reviewed-by: Ralph Böhme <s...@samba.org> ----------------------------------------------------------------------- Summary of changes: selftest/target/Samba3.pm | 4 ++ source3/modules/vfs_streams_xattr.c | 2 + source3/script/tests/test_smbclient_s3.sh | 76 +++++++++++++++++++++++++++++++ source3/smbd/dosmode.c | 22 +++++++++ 4 files changed, 104 insertions(+) Changeset truncated at 500 lines: diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 8914507..b93cbc2 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2143,6 +2143,10 @@ sub provision($$$$$$$$$) kernel oplocks = yes vfs objects = streams_xattr xattr_tdb +[streams_xattr] + copy = tmp + vfs objects = streams_xattr xattr_tdb + [compound_find] copy = tmp smbd:find async delay usec = 10000 diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c index 580ecd0..c653656 100644 --- a/source3/modules/vfs_streams_xattr.c +++ b/source3/modules/vfs_streams_xattr.c @@ -277,6 +277,7 @@ static int streams_xattr_fstat(vfs_handle_struct *handle, files_struct *fsp, sbuf->st_ex_ino = stream_inode(sbuf, io->xattr_name); sbuf->st_ex_mode &= ~S_IFMT; + sbuf->st_ex_mode &= ~S_IFDIR; sbuf->st_ex_mode |= S_IFREG; sbuf->st_ex_blocks = sbuf->st_ex_size / STAT_ST_BLOCKSIZE + 1; @@ -331,6 +332,7 @@ static int streams_xattr_stat(vfs_handle_struct *handle, smb_fname->st.st_ex_ino = stream_inode(&smb_fname->st, xattr_name); smb_fname->st.st_ex_mode &= ~S_IFMT; + smb_fname->st.st_ex_mode &= ~S_IFDIR; smb_fname->st.st_ex_mode |= S_IFREG; smb_fname->st.st_ex_blocks = smb_fname->st.st_ex_size / STAT_ST_BLOCKSIZE + 1; diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index cc0d69d..96bd5eb 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -1606,6 +1606,78 @@ EOF return 0 } +# Test xattr_stream correctly reports mode. +# BUG: https://bugzilla.samba.org/show_bug.cgi?id=13380 + +test_stream_directory_xattr() +{ + tmpfile=$PREFIX/smbclient_interactive_prompt_commands +# +# Test against streams_xattr +# + cat > $tmpfile <<EOF +deltree foo +mkdir foo +put ${PREFIX}/smbclient_interactive_prompt_commands foo:bar +setmode foo -a +allinfo foo:bar +deltree foo +quit +EOF + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/streams_xattr -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f $tmpfile + + if [ $ret != 0 ] ; then + echo "$out" + echo "failed checking attributes on xattr stream foo:bar with error $ret" + return 1 + fi + + echo "$out" | grep "attributes:.*80" + ret=$? + if [ $ret != 0 ] ; then + echo "$out" + echo "failed checking attributes on xattr stream foo:bar" + return 1 + fi + +# +# Test against streams_depot +# + cat > $tmpfile <<EOF +deltree foo +mkdir foo +put ${PREFIX}/smbclient_interactive_prompt_commands foo:bar +setmode foo -a +allinfo foo:bar +deltree foo +quit +EOF + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/tmp -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f $tmpfile + + if [ $ret != 0 ] ; then + echo "$out" + echo "failed checking attributes on depot stream foo:bar with error $ret" + return 1 + fi + + echo "$out" | grep "attributes:.*80" + ret=$? + if [ $ret != 0 ] ; then + echo "$out" + echo "failed checking attributes on depot stream foo:bar" + return 1 + fi +} + +# LOGDIR_PREFIX=test_smbclient_s3 # possibly remove old logdirs: @@ -1705,6 +1777,10 @@ testit "streams_depot can delete correctly" \ test_streams_depot_delete || \ failed=`expr $failed + 1` +testit "stream_xattr attributes" \ + test_stream_directory_xattr || \ + failed=`expr $failed + 1` + testit "follow symlinks = no" \ test_nosymlinks || \ failed=`expr $failed + 1` diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c index 8a11c8f..7ac876a 100644 --- a/source3/smbd/dosmode.c +++ b/source3/smbd/dosmode.c @@ -681,6 +681,28 @@ uint32_t dos_mode(connection_struct *conn, struct smb_filename *smb_fname) } } + /* + * According to MS-FSA a stream name does not have + * separate DOS attribute metadata, so we must return + * the DOS attribute from the base filename. With one caveat, + * a non-default stream name can never be a directory. + * + * As this is common to all streams data stores, we handle + * it here instead of inside all stream VFS modules. + * + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=13380 + */ + + if (is_ntfs_stream_smb_fname(smb_fname)) { + /* is_ntfs_stream_smb_fname() returns false for a POSIX path. */ + if (!is_ntfs_default_stream_smb_fname(smb_fname)) { + /* + * Non-default stream name, not a posix path. + */ + result &= ~(FILE_ATTRIBUTE_DIRECTORY); + } + } + if (conn->fs_capabilities & FILE_FILE_COMPRESSION) { bool compressed = false; status = dos_mode_check_compressed(conn, smb_fname, -- Samba Shared Repository