From: Volker Lendecke <vl@samba.org>
Date: Thu, 19 Nov 2020 11:38:06 +0000 (+0100)
Subject: smbd: Fix failure to check dstdir for delete on close
X-Git-Tag: samba-4.14.0rc1~574
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=382a5c4e7ec08ec9291453ffad9541ab36aca274;p=thirdparty%2Fsamba.git

smbd: Fix failure to check dstdir for delete on close

In smb2_setinfo.c the call to smbd_do_setfilepathinfo() to perform the
rename takes place while holding a share mode lock. The function
check_parent_access() called below tries to query the destination
directory's locking.tdb entry to check whether the delete on close
flag is set on the destination directory. This fails because the
file to be renamed already has the share mode entry locked, we can't
lock two share mode entries simultaneously.

Convert the check to use fetch_share_mode_unlocked(). This might
introduce races, but this whole check is racy anyway. It does not
really matter whether we do the check for delete_on_close under a lock
or not, fetch_share_mode_unlocked() retrieves a consistent status of
the locking.tdb entry at some point in time as well.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Fri Nov 20 00:20:06 UTC 2020 on sn-devel-184
---

diff --git a/selftest/knownfail.d/rename_to_del_on_close_dir b/selftest/knownfail.d/rename_to_del_on_close_dir
deleted file mode 100644
index dc450e0ddb5..00000000000
--- a/selftest/knownfail.d/rename_to_del_on_close_dir
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.libsmb.samba.tests.libsmb.LibsmbTestCase.test_RenameDstDelOnClose\(nt4_dc_smb1\)
\ No newline at end of file
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 95934e2e321..11ddfc6eb09 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -373,7 +373,12 @@ NTSTATUS check_parent_access(struct connection_struct *conn,
 		goto out;
 	}
 
-	lck = get_existing_share_mode_lock(frame, id);
+	/*
+	 * Don't take a lock here. We just need a snapshot
+	 * of the current state of delete on close and someone
+	 * else may already have a lock on this id.
+	 */
+	lck = fetch_share_mode_unlocked(frame, id);
 	if (lck == NULL) {
 		status = NT_STATUS_OK;
 		goto out;