[SCM] Samba Shared Repository - branch v3-6-test updated

Jeremy Allison jra at samba.org
Fri Apr 8 19:33:11 MDT 2011


The branch, v3-6-test has been updated
       via  37145c9 Fix bug 8072 - PANIC: create_file_acl_common frees handle two times.
       via  aa08a22 Subtle change. Microsoft SMB2 tests return different access mask than for SMB1 with raw.acls. (cherry picked from commit c6c17242d23cd03acd5adc29969177881a7d04e1)
      from  0a660c4 s3:net idmap check: fix output of an invalid record

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-6-test


- Log -----------------------------------------------------------------
commit 37145c97f59bdf0e6c97a0545f5aa9c35a6243af
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Apr 8 14:24:44 2011 -0700

    Fix bug 8072 - PANIC: create_file_acl_common frees handle two times.
    
    Caused by premature optimisation storing the parent ACL on the
    module handle instead of (correctly) on the file fsp. Previous
    code wasn't reentrant safe. This is less optimal but doesn't
    crash in the specific case :-).
    
    Jeremy.
    
    Autobuild-User: Jeremy Allison <jra at samba.org>
    Autobuild-Date: Sat Apr  9 02:05:15 CEST 2011 on sn-devel-104
    (cherry picked from commit af45636166c7a0cb87630105d18ce489e7391525)

commit aa08a226cd7bf4ec9f065d044ed2c8da38050153
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Apr 8 16:18:56 2011 -0700

    Subtle change. Microsoft SMB2 tests return different access mask than for SMB1 with raw.acls.
    (cherry picked from commit c6c17242d23cd03acd5adc29969177881a7d04e1)

-----------------------------------------------------------------------

Summary of changes:
 source3/modules/vfs_acl_common.c |   99 ++++++++++++++++++-------------------
 source3/smbd/open.c              |   13 +++++-
 2 files changed, 60 insertions(+), 52 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 827c954..f8265c5 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -484,14 +484,11 @@ static NTSTATUS inherit_new_acl(vfs_handle_struct *handle,
 				psd);
 }
 
-static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle,
+static NTSTATUS get_parent_acl_common(vfs_handle_struct *handle,
 				const char *path,
-				uint32_t access_mask,
 				struct security_descriptor **pp_parent_desc)
 {
 	char *parent_name = NULL;
-	struct security_descriptor *parent_desc = NULL;
-	uint32_t access_granted = 0;
 	NTSTATUS status;
 
 	if (!parent_dirname(talloc_tos(), path, &parent_name, NULL)) {
@@ -504,15 +501,31 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle,
 					(SECINFO_OWNER |
 					 SECINFO_GROUP |
 					 SECINFO_DACL),
-					&parent_desc);
+					pp_parent_desc);
 
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(10,("check_parent_acl_common: get_nt_acl_internal "
+		DEBUG(10,("get_parent_acl_common: get_nt_acl_internal "
 			"on directory %s for "
 			"path %s returned %s\n",
 			parent_name,
 			path,
 			nt_errstr(status) ));
+	}
+	return status;
+}
+
+static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle,
+				const char *path,
+				uint32_t access_mask,
+				struct security_descriptor **pp_parent_desc)
+{
+	char *parent_name = NULL;
+	struct security_descriptor *parent_desc = NULL;
+	uint32_t access_granted = 0;
+	NTSTATUS status;
+
+	status = get_parent_acl_common(handle, path, &parent_desc);
+	if (!NT_STATUS_IS_OK(status)) {
 		return status;
 	}
 	if (pp_parent_desc) {
@@ -536,11 +549,6 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle,
 	return NT_STATUS_OK;
 }
 
-static void free_sd_common(void **ptr)
-{
-	TALLOC_FREE(*ptr);
-}
-
 /*********************************************************************
  Check ACL on open. For new files inherit from parent directory.
 *********************************************************************/
@@ -553,7 +561,6 @@ static int open_acl_common(vfs_handle_struct *handle,
 {
 	uint32_t access_granted = 0;
 	struct security_descriptor *pdesc = NULL;
-	struct security_descriptor *parent_desc = NULL;
 	bool file_existed = true;
 	char *fname = NULL;
 	NTSTATUS status;
@@ -599,29 +606,28 @@ static int open_acl_common(vfs_handle_struct *handle,
 		 * Check the parent directory ACL will allow this.
 		 */
 		if (flags & O_CREAT) {
-			struct security_descriptor *psd = NULL;
+			struct security_descriptor *parent_desc = NULL;
+			struct security_descriptor **pp_psd = NULL;
 
 			status = check_parent_acl_common(handle, fname,
 					SEC_DIR_ADD_FILE, &parent_desc);
 			if (!NT_STATUS_IS_OK(status)) {
 				goto err;
 			}
-			/* Cache the parent security descriptor for
-			 * later use. We do have an fsp here, but to
-			 * keep the code consistent with the directory
-			 * case which doesn't, use the handle. */
 
-			/* Attach this to the conn, move from talloc_tos(). */
-			psd = (struct security_descriptor *)talloc_move(handle->conn,
-				&parent_desc);
+			/* Cache the parent security descriptor for
+			 * later use. */
 
-			if (!psd) {
+			pp_psd = VFS_ADD_FSP_EXTENSION(handle,
+					fsp,
+					struct security_descriptor *,
+					NULL);
+			if (!pp_psd) {
 				status = NT_STATUS_NO_MEMORY;
 				goto err;
 			}
-			status = NT_STATUS_NO_MEMORY;
-			SMB_VFS_HANDLE_SET_DATA(handle, psd, free_sd_common,
-				struct security_descriptor *, goto err);
+
+			*pp_psd = parent_desc;
 			status = NT_STATUS_OK;
 		}
 	}
@@ -648,30 +654,13 @@ static int mkdir_acl_common(vfs_handle_struct *handle, const char *path, mode_t
 
 	ret = vfs_stat_smb_fname(handle->conn, path, &sbuf);
 	if (ret == -1 && errno == ENOENT) {
-		struct security_descriptor *parent_desc = NULL;
-		struct security_descriptor *psd = NULL;
-
 		/* We're creating a new directory. */
 		status = check_parent_acl_common(handle, path,
-				SEC_DIR_ADD_SUBDIR, &parent_desc);
+				SEC_DIR_ADD_SUBDIR, NULL);
 		if (!NT_STATUS_IS_OK(status)) {
 			errno = map_errno_from_nt_status(status);
 			return -1;
 		}
-
-		/* Cache the parent security descriptor for
-		 * later use. We don't have an fsp here so
-		 * use the handle. */
-
-		/* Attach this to the conn, move from talloc_tos(). */
-		psd = (struct security_descriptor *)talloc_move(handle->conn,
-				&parent_desc);
-
-		if (!psd) {
-			return -1;
-		}
-		SMB_VFS_HANDLE_SET_DATA(handle, psd, free_sd_common,
-			struct security_descriptor *, return -1);
 	}
 
 	return SMB_VFS_NEXT_MKDIR(handle, path, mode);
@@ -916,6 +905,7 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 	files_struct *fsp = NULL;
 	int info;
 	struct security_descriptor *parent_sd = NULL;
+	struct security_descriptor **pp_parent_sd = NULL;
 
 	status = SMB_VFS_NEXT_CREATE_FILE(handle,
 					req,
@@ -960,13 +950,19 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 		goto out;
 	}
 
-
-	/* We must have a cached parent sd in this case.
-	 * attached to the handle. */
-
-	SMB_VFS_HANDLE_GET_DATA(handle, parent_sd,
-		struct security_descriptor,
-		goto err);
+	/* See if we have a cached parent sd, if so, use it. */
+	pp_parent_sd = (struct security_descriptor **)VFS_FETCH_FSP_EXTENSION(handle, fsp);
+	if (!pp_parent_sd) {
+		/* Must be a directory, fetch again (sigh). */
+		status = get_parent_acl_common(handle,
+				fsp->fsp_name->base_name,
+				&parent_sd);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto out;
+		}
+	} else {
+		parent_sd = *pp_parent_sd;
+	}
 
 	if (!parent_sd) {
 		goto err;
@@ -984,8 +980,9 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle,
 
   out:
 
-	/* Ensure we never leave attached data around. */
-	SMB_VFS_HANDLE_FREE_DATA(handle);
+	if (fsp) {
+		VFS_REMOVE_FSP_EXTENSION(handle, fsp);
+	}
 
 	if (NT_STATUS_IS_OK(status) && pinfo) {
 		*pinfo = info;
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index c03200e..5c449fc 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2266,7 +2266,18 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 	 * According to Samba4, SEC_FILE_READ_ATTRIBUTE is always granted,
 	 * but we don't have to store this - just ignore it on access check.
 	 */
-	fsp->access_mask = access_mask;
+	if (conn->sconn->using_smb2) {
+		/*
+		 * SMB2 doesn't return it (according to Microsoft tests).
+		 * Test Case: TestSuite_ScenarioNo009GrantedAccessTestS0
+		 * File created with access = 0x7 (Read, Write, Delete)
+		 * Query Info on file returns 0x87 (Read, Write, Delete, Read Attributes)
+		 */
+		fsp->access_mask = access_mask;
+	} else {
+		/* But SMB1 does. */
+		fsp->access_mask = access_mask | FILE_READ_ATTRIBUTES;
+	}
 
 	if (file_existed) {
 		/* stat opens on existing files don't get oplocks. */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list