Attachment 6546 Details for Bug 8211 – Updated fix for 3.5.9 - fixes issue Bjorn found.

[patch] Updated fix for 3.5.9 - fixes issue Bjorn found.

bug-8211-3.5.9.patch (text/plain), 5.72 KB, created by Jeremy Allison on 2011-06-08 21:25:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2011-06-08 21:25:10 UTC

Size: 5.72 KB

patch

obsolete

>From 6db8b829d8e02143be883cc5dd4b1ff53ae2b304 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 8 Jun 2011 10:24:02 -0700
>Subject: [PATCH 1/4] Part 1 of bugfix for #8211 - "inherit owner = yes" doesn't interact correctly with "inherit permissions = yes" and POSIX ACLs
>
>When changing ownership on a new directory make sure we
>also change the returned stat struct to have the correct uid.
>---
> source3/smbd/open.c |    3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 0d585cf..e8d1a1b 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -338,6 +338,9 @@ NTSTATUS change_dir_owner_to_parent(connection_struct *conn,
> 		  "directory %s to parent directory uid %u.\n",
> 		  fname, (unsigned int)smb_fname_parent->st.st_ex_uid ));
> 
>+	/* Ensure the uid entry is updated. */
>+	psbuf->st_ex_uid = smb_fname_parent->st.st_ex_uid;
>+
>  chdir:
> 	vfs_ChDir(conn,saved_dir);
>  out:
>-- 
>1.7.3.1
>
>
>From a08def4f150f782388fe943891324469d756183e Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 8 Jun 2011 10:17:42 -0700
>Subject: [PATCH 2/4] Part 2 of bugfix for #8211 - "inherit owner = yes" doesn't interact correctly with "inherit permissions = yes" and POSIX ACLs
>
>When changing ownership on a new file make sure we
>also change the returned stat struct to have the correct uid.
>---
> source3/smbd/open.c |    9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index e8d1a1b..aaae967 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -232,12 +232,15 @@ void change_file_owner_to_parent(connection_struct *conn,
> 			 "was %s\n", fsp_str_dbg(fsp),
> 			 (unsigned int)smb_fname_parent->st.st_ex_uid,
> 			 strerror(errno) ));
>-	}
>-
>-	DEBUG(10,("change_file_owner_to_parent: changed new file %s to "
>+	} else {
>+		DEBUG(10,("change_file_owner_to_parent: changed new file %s to "
> 		  "parent directory uid %u.\n", fsp_str_dbg(fsp),
> 		  (unsigned int)smb_fname_parent->st.st_ex_uid));
> 
>+		/* Ensure the uid entry is updated. */
>+		fsp->fsp_name->st.st_ex_uid = smb_fname_parent->st.st_ex_uid;
>+	}
>+
> 	TALLOC_FREE(smb_fname_parent);
> }
> 
>-- 
>1.7.3.1
>
>
>From f8855ba178938c5143684b08d66d236b0e4383e4 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 8 Jun 2011 10:25:33 -0700
>Subject: [PATCH 3/4] Part 3 of bugfix for #8211 - "inherit owner = yes" doesn't interact correctly with "inherit permissions = yes" and POSIX ACLs
>
>When changing ownership on a new file make sure we
>must have a valid stat struct before making the inheritance
>calls (as they may look at it), and if we make changes we
>must have a valid stat struct after them.
>---
> source3/smbd/open.c |   42 +++++++++++++++++++++++++-----------------
> 1 files changed, 25 insertions(+), 17 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index aaae967..e77284b 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -370,6 +370,7 @@ static NTSTATUS open_file(files_struct *fsp,
> 	int accmode = (flags & O_ACCMODE);
> 	int local_flags = flags;
> 	bool file_existed = VALID_STAT(fsp->fsp_name->st);
>+	bool file_created = false;
> 
> 	fsp->fh->fd = -1;
> 	errno = EPERM;
>@@ -469,23 +470,7 @@ static NTSTATUS open_file(files_struct *fsp,
> 		}
> 
> 		if ((local_flags & O_CREAT) && !file_existed) {
>-
>-			/* Inherit the ACL if required */
>-			if (lp_inherit_perms(SNUM(conn))) {
>-				inherit_access_posix_acl(conn, parent_dir,
>-							 smb_fname->base_name,
>-							 unx_mode);
>-			}
>-
>-			/* Change the owner if required. */
>-			if (lp_inherit_owner(SNUM(conn))) {
>-				change_file_owner_to_parent(conn, parent_dir,
>-							    fsp);
>-			}
>-
>-			notify_fname(conn, NOTIFY_ACTION_ADDED,
>-				     FILE_NOTIFY_CHANGE_FILE_NAME,
>-				     smb_fname->base_name);
>+			file_created = true;
> 		}
> 
> 	} else {
>@@ -595,6 +580,29 @@ static NTSTATUS open_file(files_struct *fsp,
> 			fd_close(fsp);
> 			return status;
> 		}
>+
>+		if (file_created) {
>+			/* Do all inheritance work after we've
>+			   done a successful stat call and filled
>+			   in the stat struct in fsp->fsp_name. */
>+
>+			/* Inherit the ACL if required */
>+			if (lp_inherit_perms(SNUM(conn))) {
>+				inherit_access_posix_acl(conn, parent_dir,
>+							 smb_fname->base_name,
>+							 unx_mode);
>+			}
>+
>+			/* Change the owner if required. */
>+			if (lp_inherit_owner(SNUM(conn))) {
>+				change_file_owner_to_parent(conn, parent_dir,
>+							    fsp);
>+			}
>+
>+			notify_fname(conn, NOTIFY_ACTION_ADDED,
>+				     FILE_NOTIFY_CHANGE_FILE_NAME,
>+				     smb_fname->base_name);
>+		}
> 	}
> 
> 	/*
>-- 
>1.7.3.1
>
>
>From da11f0f827be1c3658a85c0a9204309ba7c1c68e Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 8 Jun 2011 14:21:52 -0700
>Subject: [PATCH 4/4] Part 4 of bugfix for #8211 - "inherit owner = yes" doesn't interact correctly with "inherit permissions = yes" and POSIX ACLs
>
>We don't need to check mode bits as well as dev/ino to
>ensure we're in the same place.
>---
> source3/smbd/open.c |    5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index e77284b..c626ebe 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -314,10 +314,9 @@ NTSTATUS change_dir_owner_to_parent(connection_struct *conn,
> 
> 	/* Ensure we're pointing at the same place. */
> 	if (smb_fname_cwd->st.st_ex_dev != psbuf->st_ex_dev ||
>-	    smb_fname_cwd->st.st_ex_ino != psbuf->st_ex_ino ||
>-	    smb_fname_cwd->st.st_ex_mode != psbuf->st_ex_mode ) {
>+	    smb_fname_cwd->st.st_ex_ino != psbuf->st_ex_ino) {
> 		DEBUG(0,("change_dir_owner_to_parent: "
>-			 "device/inode/mode on directory %s changed. "
>+			 "device/inode on directory %s changed. "
> 			 "Refusing to chown !\n", fname ));
> 		status = NT_STATUS_ACCESS_DENIED;
> 		goto chdir;
>-- 
>1.7.3.1
>

Actions: View

Attachments on bug 8211: 6543 | 6544 | 6546 | 6547 | 6548 | 6549