[PATCH] Stop streams_depot from leaving lost-XXXX directories on file deletion.

[Jeremy Allison]

As reported (and the fix tested) by  Hemanth Thummal
from Nutanix.

Regression test included.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12384

Please review and push if happy !

Jeremy.
-------------- next part --------------
From 366e4a01e0cbb6ec3940f21ab47ec53335bd3989 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 19 Oct 2016 11:56:49 -0700
Subject: [PATCH 1/2] s3: vfs: Remove files/directories after the streams are
 deleted.

By the time we get to SMB_VFS_UNLINK/SMB_VFS_RMDIR the ACL
checks have already been done.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12384

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_streams_depot.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_streams_depot.c b/source3/modules/vfs_streams_depot.c
index 83c9d97..d874514 100644
--- a/source3/modules/vfs_streams_depot.c
+++ b/source3/modules/vfs_streams_depot.c
@@ -725,8 +725,12 @@ static int streams_depot_unlink(vfs_handle_struct *handle,
 		return -1;
 	}
 
-	ret = SMB_VFS_NEXT_UNLINK(handle, smb_fname);
-	if (ret == 0) {
+	/*
+	 * We know the unlink should succeed as the ACL
+	 * check is already done in the caller. Remove the
+	 * file *after* the streams.
+	 */
+	{
 		char *dirname = stream_dir(handle, smb_fname_base,
 					   &smb_fname_base->st, false);
 
@@ -749,6 +753,7 @@ static int streams_depot_unlink(vfs_handle_struct *handle,
 		TALLOC_FREE(dirname);
 	}
 
+	ret = SMB_VFS_NEXT_UNLINK(handle, smb_fname);
 	TALLOC_FREE(smb_fname_base);
 	return ret;
 }
@@ -787,8 +792,12 @@ static int streams_depot_rmdir(vfs_handle_struct *handle,
 		return -1;
 	}
 
-	ret = SMB_VFS_NEXT_RMDIR(handle, smb_fname_base);
-	if (ret == 0) {
+	/*
+	 * We know the rmdir should succeed as the ACL
+	 * check is already done in the caller. Remove the
+	 * directory *after* the streams.
+	 */
+	{
 		char *dirname = stream_dir(handle, smb_fname_base,
 					   &smb_fname_base->st, false);
 
@@ -811,6 +820,7 @@ static int streams_depot_rmdir(vfs_handle_struct *handle,
 		TALLOC_FREE(dirname);
 	}
 
+	ret = SMB_VFS_NEXT_RMDIR(handle, smb_fname_base);
 	TALLOC_FREE(smb_fname_base);
 	return ret;
 }
-- 
2.8.0.rc3.226.g39d4020


From db2f844a587ddf2118432f28898de5d1aad78476 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 19 Oct 2016 16:33:52 -0700
Subject: [PATCH 2/2] s3: selftest: Add test for orphan 'lost-XXX' directories
 in streams_depot.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12384

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/script/tests/test_smbclient_s3.sh | 42 +++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index 5e3db5d..b4faee1 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -1060,6 +1060,44 @@ EOF
     fi
 }
 
+# Test creating then deleting a stream file doesn't leave a lost-XXXXX directory.
+test_streams_depot_delete()
+{
+    tmpfile=$PREFIX/smbclient_interactive_prompt_commands
+    rm -rf "$LOCAL_PATH/lost-*"
+
+    cat > $tmpfile <<EOF
+put ${PREFIX}/smbclient_interactive_prompt_commands foo:bar
+del foo
+ls lost*
+quit
+EOF
+    cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/tmp -I $SERVER_IP -mSMB3 $ADDARGS < $tmpfile 2>&1'
+    eval echo "$cmd"
+    out=`eval $cmd`
+    ret=$?
+    echo "$out" >>/tmp/lookatme
+    rm -f $tmpfile
+
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "failed creating then deleting foo:bar with error $ret"
+	false
+	return
+    fi
+
+    echo "$out" | grep 'NT_STATUS_NO_SUCH_FILE listing \\lost\*'
+    ret=$?
+    if [ $ret != 0 ] ; then
+	echo "$out"
+	echo "deleting foo:bar left lost-XXX directory"
+	rm -rf "$LOCAL_PATH/lost-*"
+	false
+	return
+    fi
+}
+
+
 LOGDIR_PREFIX=test_smbclient_s3
 
 # possibly remove old logdirs:
@@ -1155,6 +1193,10 @@ testit "Ensure widelinks are restricted" \
     test_widelinks || \
     failed=`expr $failed + 1`
 
+testit "streams_depot can delete correctly" \
+    test_streams_depot_delete || \
+    failed=`expr $failed + 1`
+
 testit "rm -rf $LOGDIR" \
     rm -rf $LOGDIR || \
     failed=`expr $failed + 1`
-- 
2.8.0.rc3.226.g39d4020