From c49179c9679d923ababaa3d7b6cd5f6b2cfc81c3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 8 Apr 2011 15:25:18 -0700 Subject: [PATCH] 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. --- source3/modules/vfs_acl_common.c | 111 ++++++++++++++++++------------------- 1 files changed, 54 insertions(+), 57 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index f1884f5..a5ea505 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; -- 1.7.3.1