s3: smbd: Don't loop infinitely on bad-symlink resolution.

In the FILE_OPEN_IF case we have O_CREAT, but not
O_EXCL. Previously we went into a loop trying first
~(O_CREAT|O_EXCL), and if that returned ENOENT
try (O_CREAT|O_EXCL). We kept looping indefinately
until we got an error, or the file was created or
opened.

The big problem here is dangling symlinks. Opening
without O_NOFOLLOW means both bad symlink
and missing path return -1, ENOENT from open(). As POSIX
is pathname based it's not possible to tell
the difference between these two cases in a
non-racy way, so change to try only two attempts before
giving up.

We don't have this problem for the O_NOFOLLOW
case as we just return NT_STATUS_OBJECT_PATH_NOT_FOUND
mapped from the ELOOP POSIX error and immediately
returned.

Unroll the loop logic to two tries instead.

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

Pair-programmed-with: Ralph Boehme <slow@samba.org>

Signed-off-by: Jeremy Allison <jra@samba.org>
Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit 10c3e3923022485c720f322ca4f0aca5d7501310)

1 files changed, 56 insertions, 48 deletions

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 0184a00063a..25cf4177e4a 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -621,7 +621,9 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
 			bool *file_created)
 {
 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
+	NTSTATUS retry_status;
 	bool file_existed = VALID_STAT(fsp->fsp_name->st);
+	int curr_flags;
 
 	*file_created = false;
 
@@ -653,59 +655,65 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
 	 * we can never call O_CREAT without O_EXCL. So if
 	 * we think the file existed, try without O_CREAT|O_EXCL.
 	 * If we think the file didn't exist, try with
-	 * O_CREAT|O_EXCL. Keep bouncing between these two
-	 * requests until either the file is created, or
-	 * opened. Either way, we keep going until we get
-	 * a returnable result (error, or open/create).
+	 * O_CREAT|O_EXCL.
+	 *
+	 * The big problem here is dangling symlinks. Opening
+	 * without O_NOFOLLOW means both bad symlink
+	 * and missing path return -1, ENOENT from open(). As POSIX
+	 * is pathname based it's not possible to tell
+	 * the difference between these two cases in a
+	 * non-racy way, so change to try only two attempts before
+	 * giving up.
+	 *
+	 * We don't have this problem for the O_NOFOLLOW
+	 * case as it just returns NT_STATUS_OBJECT_PATH_NOT_FOUND
+	 * mapped from the ELOOP POSIX error.
 	 */
 
-	while(1) {
-		int curr_flags = flags;
+	curr_flags = flags;
 
-		if (file_existed) {
-			/* Just try open, do not create. */
-			curr_flags &= ~(O_CREAT);
-			status = fd_open(conn, fsp, curr_flags, mode);
-			if (NT_STATUS_EQUAL(status,
-					NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
-				/*
-				 * Someone deleted it in the meantime.
-				 * Retry with O_EXCL.
-				 */
-				file_existed = false;
-				DEBUG(10,("fd_open_atomic: file %s existed. "
-					"Retry.\n",
-					smb_fname_str_dbg(fsp->fsp_name)));
-					continue;
-			}
-		} else {
-			/* Try create exclusively, fail if it exists. */
-			curr_flags |= O_EXCL;
-			status = fd_open(conn, fsp, curr_flags, mode);
-			if (NT_STATUS_EQUAL(status,
-					NT_STATUS_OBJECT_NAME_COLLISION)) {
-				/*
-				 * Someone created it in the meantime.
-				 * Retry without O_CREAT.
-				 */
-				file_existed = true;
-				DEBUG(10,("fd_open_atomic: file %s "
-					"did not exist. Retry.\n",
-					smb_fname_str_dbg(fsp->fsp_name)));
-				continue;
-			}
-			if (NT_STATUS_IS_OK(status)) {
-				/*
-				 * Here we've opened with O_CREAT|O_EXCL
-				 * and got success. We *know* we created
-				 * this file.
-				 */
-				*file_created = true;
-			}
+	if (file_existed) {
+		curr_flags &= ~(O_CREAT);
+		retry_status = NT_STATUS_OBJECT_NAME_NOT_FOUND;
+	} else {
+		curr_flags |= O_EXCL;
+		retry_status = NT_STATUS_OBJECT_NAME_COLLISION;
+	}
+
+	status = fd_open(conn, fsp, curr_flags, mode);
+	if (NT_STATUS_IS_OK(status)) {
+		if (!file_existed) {
+			*file_created = true;
 		}
-		/* Create is done, or failed. */
-		break;
+		return NT_STATUS_OK;
 	}
+	if (!NT_STATUS_EQUAL(status, retry_status)) {
+		return status;
+	}
+
+	curr_flags = flags;
+
+	/*
+	 * Keep file_existed up to date for clarity.
+	 */
+	if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) {
+		file_existed = false;
+		curr_flags |= O_EXCL;
+		DBG_DEBUG("file %s did not exist. Retry.\n",
+			smb_fname_str_dbg(fsp->fsp_name));
+	} else {
+		file_existed = true;
+		curr_flags &= ~(O_CREAT);
+		DBG_DEBUG("file %s existed. Retry.\n",
+			smb_fname_str_dbg(fsp->fsp_name));
+	}
+
+	status = fd_open(conn, fsp, curr_flags, mode);
+
+	if (NT_STATUS_IS_OK(status) && (!file_existed)) {
+		*file_created = true;
+	}
+
 	return status;
 }