Attachment 12925 Details for Bug 12572 – git-am fix for master

[patch] git-am fix for master

bug-12572 (text/plain), 4.26 KB, created by Jeremy Allison on 2017-02-13 22:25:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2017-02-13 22:25:51 UTC

Size: 4.26 KB

patch

obsolete

>From b8d76869e68e16a87badf1e3ab6c538311be9062 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 13 Feb 2017 14:12:48 -0800
>Subject: [PATCH 1/2] s3: smbd: Don't loop infinitely on bad-symlink
> resolution.
>
>If we think the file didn't exist, try the open with
>O_CREAT|O_EXCL. Bounce between these two
>requests until either the file is created, or
>opened. Either way, we try 5 times until we
>a returnable result (error, or open/create).
>
>The big problem are symlinks here. Opening
>without O_NOFOLLOW means both bad symlink
>and missing path return -1, ENOENT. As POSIX
>is pathname based it's not possible to tell
>the difference between these two cases in a
>non-racy way, so use a heuristic of 5
>attempts before giving up. We must use
>an odd number as the terminating loop
>heuristic as we think the file exists
>and must return NT_STATUS_OBJECT_NAME_NOT_FOUND
>for both the file got deleted and bad
>symlink case.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12572
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 931d76d..17757a0 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -641,6 +641,7 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
> {
> 	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
> 	bool file_existed = VALID_STAT(fsp->fsp_name->st);
>+	unsigned int loop_counter = 0;
> 
> 	*file_created = false;
> 
>@@ -674,11 +675,24 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
> 	 * 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).
>+	 * opened. Either way, try 5 times before giving
>+	 * up and returning the last error we got.
>+	 *
>+	 * The big problem are symlinks here. Opening
>+	 * without O_NOFOLLOW means both bad symlink
>+	 * and missing path return -1, ENOENT. As POSIX
>+	 * is pathname based it's not possible to tell
>+	 * the difference between these two cases in a
>+	 * non-racy way, so use a heuristic of 5
>+	 * attempts before giving up. We must use
>+	 * an odd number as the terminating loop
>+	 * heuristic as we think the file exists
>+	 * and must return NT_STATUS_OBJECT_NAME_NOT_FOUND
>+	 * for both the file got deleted and bad
>+	 * symlink case.
> 	 */
> 
>-	while(1) {
>+	for(loop_counter = 0; loop_counter < 5; loop_counter++) {
> 		int curr_flags = flags;
> 
> 		if (file_existed) {
>-- 
>2.7.4
>
>
>From 4e998e3ea63ec5008b1cade37fba678d38b83b22 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 13 Feb 2017 14:20:55 -0800
>Subject: [PATCH 2/2] s3: smbd: Add optimization for O_NOFOLLOW case.
>
>If the underlying open returned ELOOP, this is mapped into
>NT_STATUS_OBJECT_NAME_NOT_FOUND. However, the path did exist,
>it's just a bad symlink with O_NOFOLLOW specified in
>the open flags. Return the error in this case.
>
>If we treat it as a normal NT_STATUS_OBJECT_NAME_NOT_FOUND we will
>keep the server in the loop here, so this is an optimization for
>the O_NOFOLLOW case.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12572
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 17757a0..587596b 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -718,6 +718,24 @@ static NTSTATUS fd_open_atomic(struct connection_struct *conn,
> 			if (NT_STATUS_EQUAL(status,
> 					NT_STATUS_OBJECT_NAME_COLLISION)) {
> 				/*
>+				 * Optimization. If the underlying
>+				 * open returned ELOOP, this is mapped into
>+				 * NT_STATUS_OBJECT_NAME_NOT_FOUND. However,
>+				 * the path did exist, it's just a bad
>+				 * symlink with O_NOFOLLOW specified in
>+				 * the open flags. Return the error in this
>+				 * case.
>+				 *
>+				 * If we treat it as a normal
>+				 * NT_STATUS_OBJECT_NAME_NOT_FOUND we will
>+				 * keep the server in the loop here, so this
>+				 * is an optimization for the O_NOFOLLOW case.
>+				 */
>+				if (errno == ELOOP) {
>+					return status;
>+				}
>+
>+				/*
> 				 * Someone created it in the meantime.
> 				 * Retry without O_CREAT.
> 				 */
>-- 
>2.7.4
>

Actions: View

Attachments on bug 12572: 12925 | 12928 | 12929 | 12942 | 12946 | 12948 | 12949