Attachment 10286 Details for Bug 10809 – Full set of patches (3 of them) that went into master.

[patch] Full set of patches (3 of them) that went into master.

bug-10809 (text/plain), 5.06 KB, created by Jeremy Allison on 2014-09-12 23:19:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2014-09-12 23:19:55 UTC

Size: 5.06 KB

patch

obsolete

>From e07d71bcfdee39c3f2adb384ab17c50da07af997 Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>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 <obnox@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> 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;
>+				}
> 			}
> 		}
> 
>-- 
>2.1.0.rc2.206.gedb03e5
>
>
>From d7cc859cdf327a3f6bedb9600449b238519ac67a Mon Sep 17 00:00:00 2001
>From: Michael Adam <obnox@samba.org>
>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 <jra@samba.org>.
>
>Signed-off-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>---
> 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",
>-- 
>2.1.0.rc2.206.gedb03e5
>
>
>From 37d449e6dd5b38a62ea8a141136aebd44f6de292 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>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 <jra@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>Reviewed-by: Michael Adam <obnox@samba.org>
>
>Autobuild-User(master): Michael Adam <obnox@samba.org>
>Autobuild-Date(master): Thu Sep 11 22:29:22 CEST 2014 on sn-devel-104
>---
> 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)
>-- 
>2.1.0.rc2.206.gedb03e5
>

Actions: View

Attachments on bug 10809: 10281 | 10282 | 10286 | 10298