From b8d76869e68e16a87badf1e3ab6c538311be9062 Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 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 --- 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