From f1afad766261e10dd1cbd66196ce3b703422f2f2 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 10 Sep 2014 00:31:25 +0200 Subject: [PATCH 1/3] s3:smbd: fix a race in open code The race is when a file vanishes between existence check and acl check. In this case, open_file_ncreate() returns OBJECT_NAME_NOT_FOUND even if the create was called with disposition OPEN_IF. But in this case, the file should be created. Signed-off-by: Michael Adam Reviewed-by: Jeremy Allison (cherry picked from commit 8ae8c63da19459fd4f1166e11406da2c919b7ed0) BUG: https://bugzilla.samba.org/show_bug.cgi?id=10809 --- source3/smbd/open.c | 59 ++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 16d4307..cc6e080 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -818,24 +818,49 @@ static NTSTATUS open_file(files_struct *fsp, smb_fname, false, access_mask); - } else if (local_flags & O_CREAT){ - status = check_parent_access(conn, - smb_fname, - SEC_DIR_ADD_FILE); - } else { - /* File didn't exist and no O_CREAT. */ - return NT_STATUS_OBJECT_NAME_NOT_FOUND; + + if (!NT_STATUS_IS_OK(status)) { + DEBUG(10, ("open_file: " + "smbd_check_access_rights " + "on file %s returned %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status))); + } + + if (!NT_STATUS_IS_OK(status) && + !NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_NOT_FOUND)) + { + return status; + } + + if (!NT_STATUS_IS_OK(status)) { + DEBUG(10, ("open_file: " + "file %s vanished since we " + "checked for existence.\n", + smb_fname_str_dbg(smb_fname))); + file_existed = false; + SET_STAT_INVALID(fsp->fsp_name->st); + } } - if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("open_file: " - "%s on file " - "%s returned %s\n", - file_existed ? - "smbd_check_access_rights" : - "check_parent_access", - smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); - return status; + + if (!file_existed) { + if (!(local_flags & O_CREAT)) { + /* File didn't exist and no O_CREAT. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + + status = check_parent_access(conn, + smb_fname, + SEC_DIR_ADD_FILE); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(10, ("open_file: " + "check_parent_access on " + "file %s returned %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status) )); + return status; + } } } -- 1.9.1 From 5e61317b59654eee5807996cef13ca0b761f23db Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 10 Sep 2014 21:58:04 +0200 Subject: [PATCH 2/3] s3:smbd:open_file: use a more natural check. As suggested by Jeremy Allison . Signed-off-by: Michael Adam Reviewed-by: Jeremy Allison (cherry picked from commit 9da09b52e8cc0453e694d85fc2bd82994138e20b) BUG: https://bugzilla.samba.org/show_bug.cgi?id=10809 --- source3/smbd/open.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index cc6e080..a2733dc 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -834,7 +834,9 @@ static NTSTATUS open_file(files_struct *fsp, return status; } - if (!NT_STATUS_IS_OK(status)) { + if (NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_NOT_FOUND)) + { DEBUG(10, ("open_file: " "file %s vanished since we " "checked for existence.\n", -- 1.9.1 From a6a235963a8a8839d5787ea5693a9f277b419e3a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 11 Sep 2014 10:03:01 -0700 Subject: [PATCH 3/3] s3: smbd - open logic fix. As we atomically create using O_CREAT|O_EXCL, then if new_file_created is true, then file_existed *MUST* have been false (even if the file was previously detected as being there. We use the variable file_existed again in logic below this statement, so we must set file_existed = false, if new_file_created returns are true from open_file(). Based on a fix from Michael Adam. BUG: https://bugzilla.samba.org/show_bug.cgi?id=10809 Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam Reviewed-by: Michael Adam Autobuild-User(master): Michael Adam Autobuild-Date(master): Thu Sep 11 22:29:22 CEST 2014 on sn-devel-104 (cherry picked from commit 518247bf80372eb003cb67036b9d9e7fe8aac303) --- source3/smbd/open.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index a2733dc..6bb37e9 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2444,6 +2444,17 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, return fsp_open; } + if (new_file_created) { + /* + * As we atomically create using O_CREAT|O_EXCL, + * then if new_file_created is true, then + * file_existed *MUST* have been false (even + * if the file was previously detected as being + * there). + */ + file_existed = false; + } + if (file_existed && !check_same_dev_ino(&saved_stat, &smb_fname->st)) { /* * The file did exist, but some other (local or NFS) -- 1.9.1