From: Ralph Boehme Date: Tue, 23 Aug 2016 15:07:20 +0000 (+0200) Subject: vfs_acl_common: simplify ACL logic, cleanup and talloc hierarchy X-Git-Tag: samba-4.3.12~39 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8da92276aa5c36e9431bb618cde96a2a2c77ba35;p=thirdparty%2Fsamba.git vfs_acl_common: simplify ACL logic, cleanup and talloc hierarchy No change in behaviour (hopefully! :-). This paves the way for moving the ACL blob validation to a helper function in the next commit. Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (backported from commit 335527c647331148927feea2a7ae2f2c88986bc6) --- diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 64ee1ec730b..112874fe6f6 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -488,13 +488,16 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE]; uint8_t hash_tmp[XATTR_SD_HASH_SIZE]; uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE]; + struct security_descriptor *psd = NULL; struct security_descriptor *psd_blob = NULL; struct security_descriptor *psd_fs = NULL; bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), ACL_MODULE_NAME, "ignore system acls", false); - TALLOC_CTX *frame = talloc_stackframe(); + char *sys_acl_blob_description = NULL; + DATA_BLOB sys_acl_blob = { 0 }; + bool psd_is_from_fs = false; if (fsp && name == NULL) { name = fsp->fsp_name->base_name; @@ -502,11 +505,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); - status = get_acl_blob(frame, handle, fsp, name, &blob); + status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n", nt_errstr(status))); - psd_blob = NULL; goto out; } else { status = parse_acl_blob(&blob, mem_ctx, &psd_blob, @@ -514,17 +516,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("parse_acl_blob returned %s\n", nt_errstr(status))); - psd_blob = NULL; + TALLOC_FREE(blob.data); goto out; } } - /* Ensure we don't leak psd_blob if we don't choose it. - * - * We don't allocate it onto frame as it is preferred not to - * steal from a talloc pool. - */ - talloc_steal(frame, psd_blob); + TALLOC_FREE(blob.data); /* determine which type of xattr we got */ switch (xattr_version) { @@ -534,10 +531,14 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, * require confirmation of the hash. In particular, * the NTVFS file server uses version 1, but * 'samba-tool ntacl' can set these as well */ + psd = psd_blob; + psd_blob = NULL; goto out; case 3: case 4: if (ignore_file_system_acl) { + psd = psd_blob; + psd_blob = NULL; goto out; } @@ -566,20 +567,18 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, case 4: { int ret; - char *sys_acl_blob_description; - DATA_BLOB sys_acl_blob; if (fsp) { /* Get the full underlying sd, then hash. */ ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FD(handle, fsp, - frame, + mem_ctx, &sys_acl_blob_description, &sys_acl_blob); } else { /* Get the full underlying sd, then hash. */ ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FILE(handle, name, - frame, + mem_ctx, &sys_acl_blob_description, &sys_acl_blob); } @@ -589,16 +588,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, if (ret == 0) { status = hash_blob_sha256(sys_acl_blob, sys_acl_hash_tmp); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(frame); - return status; + goto fail; } + TALLOC_FREE(sys_acl_blob_description); + TALLOC_FREE(sys_acl_blob.data); + if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0], XATTR_SD_HASH_SIZE) == 0) { /* Hash matches, return blob sd. */ DEBUG(10, ("get_nt_acl_internal: blob hash " "matches for file %s\n", - name )); + name)); + psd = psd_blob; + psd_blob = NULL; goto out; } } @@ -627,21 +630,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "returned %s\n", name, nt_errstr(status))); - TALLOC_FREE(frame); - return status; + goto fail; } - /* Ensure we don't leak psd_fs if we don't choose it. - * - * We don't allocate it onto frame as it is preferred not to - * steal from a talloc pool. - */ - talloc_steal(frame, psd_fs); - status = hash_sd_sha256(psd_fs, hash_tmp); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(psd_blob); - psd_blob = psd_fs; + psd = psd_fs; + psd_fs = NULL; + psd_is_from_fs = true; goto out; } @@ -649,7 +646,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, /* Hash matches, return blob sd. */ DEBUG(10, ("get_nt_acl_internal: blob hash " "matches for file %s\n", - name )); + name)); + psd = psd_blob; + psd_blob = NULL; goto out; } @@ -666,11 +665,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, } TALLOC_FREE(psd_blob); - psd_blob = psd_fs; + psd = psd_fs; + psd_fs = NULL; + psd_is_from_fs = true; } - out: - if (psd_blob == NULL) { +out: + if (psd == NULL) { /* Get the full underlying sd, as we failed to get the * blob for the hash, or the revision/hash type wasn't * known */ @@ -679,13 +680,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, fsp, security_info, mem_ctx, - &psd_fs); + &psd); } else { status = SMB_VFS_NEXT_GET_NT_ACL(handle, name, security_info, mem_ctx, - &psd_fs); + &psd); } if (!NT_STATUS_IS_OK(status)) { @@ -693,24 +694,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "returned %s\n", name, nt_errstr(status))); - TALLOC_FREE(frame); - return status; + goto fail; } - /* Ensure we don't leak psd_fs if we don't choose it. - * - * We don't allocate it onto frame as it is preferred not to - * steal from a talloc pool. - */ - talloc_steal(frame, psd_fs); - psd_blob = psd_fs; + psd_is_from_fs = true; } - if (psd_blob != psd_fs) { - /* We're returning the blob, throw - * away the filesystem SD. */ - TALLOC_FREE(psd_fs); - } else { + if (psd_is_from_fs) { SMB_STRUCT_STAT sbuf; SMB_STRUCT_STAT *psbuf = &sbuf; bool is_directory = false; @@ -722,8 +712,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, if (fsp) { status = vfs_stat_fsp(fsp); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(frame); - return status; + goto fail; } psbuf = &fsp->fsp_name->st; } else { @@ -748,72 +737,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, name, &sbuf); if (ret == -1) { - TALLOC_FREE(frame); - return map_nt_error_from_unix(errno); + status = map_nt_error_from_unix(errno); + goto fail; } } is_directory = S_ISDIR(psbuf->st_ex_mode); if (ignore_file_system_acl) { - TALLOC_FREE(psd_fs); + TALLOC_FREE(psd); status = make_default_filesystem_acl(mem_ctx, name, psbuf, - &psd_blob); + &psd); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(frame); - return status; + goto fail; } } else { if (is_directory && - !sd_has_inheritable_components(psd_blob, + !sd_has_inheritable_components(psd, true)) { status = add_directory_inheritable_components( handle, name, psbuf, - psd_blob); + psd); if (!NT_STATUS_IS_OK(status)) { - TALLOC_FREE(frame); - return status; + goto fail; } } /* The underlying POSIX module always sets the ~SEC_DESC_DACL_PROTECTED bit, as ACLs can't be inherited in this way under POSIX. Remove it for Windows-style ACLs. */ - psd_blob->type &= ~SEC_DESC_DACL_PROTECTED; + psd->type &= ~SEC_DESC_DACL_PROTECTED; } } if (!(security_info & SECINFO_OWNER)) { - psd_blob->owner_sid = NULL; + psd->owner_sid = NULL; } if (!(security_info & SECINFO_GROUP)) { - psd_blob->group_sid = NULL; + psd->group_sid = NULL; } if (!(security_info & SECINFO_DACL)) { - psd_blob->type &= ~SEC_DESC_DACL_PRESENT; - psd_blob->dacl = NULL; + psd->type &= ~SEC_DESC_DACL_PRESENT; + psd->dacl = NULL; } if (!(security_info & SECINFO_SACL)) { - psd_blob->type &= ~SEC_DESC_SACL_PRESENT; - psd_blob->sacl = NULL; + psd->type &= ~SEC_DESC_SACL_PRESENT; + psd->sacl = NULL; } - TALLOC_FREE(blob.data); - if (DEBUGLEVEL >= 10) { DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n", name)); - NDR_PRINT_DEBUG(security_descriptor, psd_blob); + NDR_PRINT_DEBUG(security_descriptor, psd); } - /* The VFS API is that the ACL is expected to be on mem_ctx */ - *ppdesc = talloc_move(mem_ctx, &psd_blob); + *ppdesc = psd; - TALLOC_FREE(frame); return NT_STATUS_OK; + +fail: + TALLOC_FREE(psd); + TALLOC_FREE(psd_blob); + TALLOC_FREE(psd_fs); + TALLOC_FREE(sys_acl_blob_description); + TALLOC_FREE(sys_acl_blob.data); + return status; } /*********************************************************************