The branch, v3-5-test has been updated
via 23e6f41 Fix bug 8072 - PANIC: create_file_acl_common frees handle
two times.
from fae43d2 Fix bug 8088 - rpccli_samr_chng_pswd_auth_crap segfaults if
any input blobs are null.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-5-test
- Log -----------------------------------------------------------------
commit 23e6f41ec923e2d3b4684ee646c8cd29506d787a
Author: Jeremy Allison <[email protected]>
Date: Fri Apr 8 15:25:18 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.
-----------------------------------------------------------------------
Summary of changes:
source3/modules/vfs_acl_common.c | 111 ++++++++++++++++++-------------------
1 files changed, 54 insertions(+), 57 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 3f02e7f..2203749 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -479,14 +479,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)) {
@@ -496,20 +493,39 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct
*handle,
status = get_nt_acl_internal(handle,
NULL,
parent_name,
- (OWNER_SECURITY_INFORMATION |
- GROUP_SECURITY_INFORMATION |
- DACL_SECURITY_INFORMATION),
- &parent_desc);
+ (SECINFO_OWNER |
+ SECINFO_GROUP |
+ SECINFO_DACL),
+ 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) {
+ *pp_parent_desc = parent_desc;
+ }
status = smb1_file_se_access_check(handle->conn,
parent_desc,
handle->conn->server_info->ptok,
@@ -525,17 +541,9 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct
*handle,
nt_errstr(status) ));
return status;
}
- if (pp_parent_desc) {
- *pp_parent_desc = parent_desc;
- }
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.
*********************************************************************/
@@ -548,7 +556,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;
@@ -594,29 +601,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;
}
}
@@ -643,30 +649,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);
@@ -909,6 +898,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,
@@ -952,13 +942,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;
@@ -976,8 +972,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;
--
Samba Shared Repository
