samba - Aug 2016 - Issue with acl_xattr:ignore system acls in 4.5rc2

On Fri, Aug 26, 2016 at 04:03:49PM -0700, Jeremy Allison
wrote:
> On Fri, Aug 26, 2016 at 02:46:19PM -0700, Jeremy Allison via samba wrote:
> > On Fri, Aug 26, 2016 at 06:44:05PM +0200, Ralph Böhme wrote:
> > > 
> > > Cheerio!
> > > -slow
> > 
> > Still reviewing this - but a few things that will need changing:
> > 
> > When adding the validate_nt_acl_blob() function in
> > [PATCH 06/12] vfs_acl_common: move the ACL blob validation to a helper
function
> > 
> > this makes some of the existing function names in debug statements
> > incorrect.
thanks for spotting this! 
> > 
> > Eg.  validate_nt_acl_blob() has debug statements:
> 
> How about adding this on top - or squashing into it ?
Updated patchset attached. My patches now (hopefully) update all DEBUG
calls they touch. New patch on-top that does the rest of the file,
using the new DBG_LEVEL macros in all places.

Cheerio!
-slow
-------------- next part --------------

On Sat, Aug 27, 2016 at 10:14:57AM +0200, Ralph Böhme via samba
wrote:
> On Fri, Aug 26, 2016 at 04:03:49PM -0700, Jeremy Allison wrote:
> > On Fri, Aug 26, 2016 at 02:46:19PM -0700, Jeremy Allison via samba
wrote:
> > > On Fri, Aug 26, 2016 at 06:44:05PM +0200, Ralph Böhme wrote:
> > > > 
> > > > Cheerio!
> > > > -slow
> > > 
> > > Still reviewing this - but a few things that will need changing:
> > > 
> > > When adding the validate_nt_acl_blob() function in
> > > [PATCH 06/12] vfs_acl_common: move the ACL blob validation to a
helper function
> > > 
> > > this makes some of the existing function names in debug
statements
> > > incorrect.
> 
> thanks for spotting this! 
> 
> > > 
> > > Eg.  validate_nt_acl_blob() has debug statements:
> > 
> > How about adding this on top - or squashing into it ?
> 
> Updated patchset attached. My patches now (hopefully) update all DEBUG
> calls they touch. New patch on-top that does the rest of the file,
> using the new DBG_LEVEL macros in all places.
...and this one even has bug urls in all commit messages. Sorry for
forgetting this in the previous version.

Cheerio!
-slow
-------------- next part --------------

On Sat, Aug 27, 2016 at 12:46:12PM +0200, Ralph Böhme via samba
wrote:
> 
> ...and this one even has bug urls in all commit messages. Sorry for
> forgetting this in the previous version.
Juuuusttt *one* leetle change, sorry :-).

I was following the changes to the talloc heirarchy in the
code and realized that adding the following change made it
much clearer (at least to me).

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2163a75..870e6da 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -625,7 +625,7 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
                                     vfs_handle_struct *handle,
                                     files_struct *fsp,
                                     const struct smb_filename *smb_fname,
-                                    DATA_BLOB *blob,
+                                    const DATA_BLOB *blob,
                                     struct security_descriptor **ppsd,
                                     bool *psd_is_from_fs)
 {

Making the blob pointer parameter to validate_nt_acl_blob() const helps
be realize it's an 'in' parameter the function should be messing
with
(and extra const is always good, right ?).

Also, as you've re-written most of this can I add your name as a

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2163a75..fa65fd1 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -4,6 +4,7 @@
  *
  * Copyright (C) Volker Lendecke, 2008
  * Copyright (C) Jeremy Allison, 2009
+ * Copyright (C) Ralph Böhme, 2016
  *

to the top as well ? If you're good with these changes squashed
in, 'Reviewed-by: Jeremy Allison <jra at samba.org>'

Let me know and I'll push.

Cheers,

	Jeremy.
> From 874b48593225e204e62c9b998a23b021c27dd023 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 24 Aug 2016 10:04:24 +0200
> Subject: [PATCH 01/13] Revert "vfs_acl_xattr: objects without NT ACL
xattr"
> 
> This reverts commit 961c4b591bb102751079d9cc92d7aa1c37f1958c.
> 
> Subsequent commits will add the same functionality as an optional
> feature.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 43
++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 2fda938e..a287945 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -379,10 +379,12 @@ static NTSTATUS
make_default_filesystem_acl(TALLOC_CTX *ctx,
>  	gid_to_sid(&group_sid, psbuf->st_ex_gid);
>  
>  	/*
> -	 * We provide 2 ACEs:
> -	 * - Owner
> -	 * - NT System
> -	 */
> +	 We provide up to 4 ACEs
> +		- Owner
> +		- Group
> +		- Everyone
> +		- NT System
> +	*/
>  
>  	if (mode & S_IRUSR) {
>  		if (mode & S_IWUSR) {
> @@ -402,6 +404,39 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX
*ctx,
>  			0);
>  	idx++;
>  
> +	access_mask = 0;
> +	if (mode & S_IRGRP) {
> +		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
> +	}
> +	if (mode & S_IWGRP) {
> +		/* note that delete is not granted - this matches posix behaviour */
> +		access_mask |= SEC_RIGHTS_FILE_WRITE;
> +	}
> +	if (access_mask) {
> +		init_sec_ace(&aces[idx],
> +			&group_sid,
> +			SEC_ACE_TYPE_ACCESS_ALLOWED,
> +			access_mask,
> +			0);
> +		idx++;
> +	}
> +
> +	access_mask = 0;
> +	if (mode & S_IROTH) {
> +		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
> +	}
> +	if (mode & S_IWOTH) {
> +		access_mask |= SEC_RIGHTS_FILE_WRITE;
> +	}
> +	if (access_mask) {
> +		init_sec_ace(&aces[idx],
> +			&global_sid_World,
> +			SEC_ACE_TYPE_ACCESS_ALLOWED,
> +			access_mask,
> +			0);
> +		idx++;
> +	}
> +
>  	init_sec_ace(&aces[idx],
>  			&global_sid_System,
>  			SEC_ACE_TYPE_ACCESS_ALLOWED,
> -- 
> 2.7.4
> 
> 
> From 55d74651c664d210a55b6833dee575dc1b101271 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 23 Aug 2016 13:08:12 +0200
> Subject: [PATCH 02/13] vfs_acl_common: rename psd to psd_blob in
>  get_nt_acl_internal()
> 
> This makes it explicit where the SD is originating from. No change in
> behaviour.
> 
> This just paves the way for a later change that will simplify the whole
> logic and talloc hierarchy, therefor this also strictly renames the
> occurences after the out label.
> 
> Logically, behind the out label, we're dealing with a variable that
> points to what we're going to return, so the name psd_blob is
> misleading, but I'm desperately trying to avoid logic changes in this
> commit and therefor I'm just strictly renaming.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 58
++++++++++++++++++++--------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index a287945..1adc875 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -488,7 +488,7 @@ 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 *pdesc_next = NULL;
>  	const struct smb_filename *smb_fname = NULL;
>  	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
> @@ -509,25 +509,25 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	if (!NT_STATUS_IS_OK(status)) {
>  		DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
>  			nt_errstr(status)));
> -		psd = NULL;
> +		psd_blob = NULL;
>  		goto out;
>  	} else {
> -		status = parse_acl_blob(&blob, mem_ctx, &psd,
> +		status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
>  					&hash_type, &xattr_version, &hash[0],
&sys_acl_hash[0]);
>  		if (!NT_STATUS_IS_OK(status)) {
>  			DEBUG(10, ("parse_acl_blob returned %s\n",
>  				   nt_errstr(status)));
> -			psd = NULL;
> +			psd_blob = NULL;
>  			goto out;
>  		}
>  	}
>  
> -	/* Ensure we don't leak psd if we don't choose it.
> +	/* 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);
> +	talloc_steal(frame, psd_blob);
>  
>  	/* determine which type of xattr we got */
>  	switch (xattr_version) {
> @@ -550,8 +550,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			   "mismatch (%u) for file %s\n",
>  			   (unsigned int)hash_type,
>  			   smb_fname->base_name));
> -		TALLOC_FREE(psd);
> -		psd = NULL;
> +		TALLOC_FREE(psd_blob);
> +		psd_blob = NULL;
>  		goto out;
>  	}
>  
> @@ -561,8 +561,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			   "(%u) unexpected for file %s\n",
>  			   (unsigned int)hash_type,
>  			   smb_fname->base_name));
> -		TALLOC_FREE(psd);
> -		psd = NULL;
> +		TALLOC_FREE(psd_blob);
> +		psd_blob = NULL;
>  		goto out;
>  	}
>  
> @@ -645,8 +645,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  
>  		status = hash_sd_sha256(pdesc_next, hash_tmp);
>  		if (!NT_STATUS_IS_OK(status)) {
> -			TALLOC_FREE(psd);
> -			psd = pdesc_next;
> +			TALLOC_FREE(psd_blob);
> +			psd_blob = pdesc_next;
>  			goto out;
>  		}
>  
> @@ -670,12 +670,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
>  		}
>  
> -		TALLOC_FREE(psd);
> -		psd = pdesc_next;
> +		TALLOC_FREE(psd_blob);
> +		psd_blob = pdesc_next;
>  	}
>    out:
>  
> -	if (psd == NULL) {
> +	if (psd_blob == NULL) {
>  		/* Get the full underlying sd, as we failed to get the
>  		 * blob for the hash, or the revision/hash type wasn't
>  		 * known */
> @@ -708,10 +708,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		 * steal from a talloc pool.
>  		 */
>  		talloc_steal(frame, pdesc_next);
> -		psd = pdesc_next;
> +		psd_blob = pdesc_next;
>  	}
>  
> -	if (psd != pdesc_next) {
> +	if (psd_blob != pdesc_next) {
>  		/* We're returning the blob, throw
>   		 * away the filesystem SD. */
>  		TALLOC_FREE(pdesc_next);
> @@ -764,20 +764,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			status = make_default_filesystem_acl(mem_ctx,
>  						smb_fname->base_name,
>  						psbuf,
> -						&psd);
> +						&psd_blob);
>  			if (!NT_STATUS_IS_OK(status)) {
>  				TALLOC_FREE(frame);
>  				return status;
>  			}
>  		} else {
>  			if (is_directory &&
> -				!sd_has_inheritable_components(psd,
> +				!sd_has_inheritable_components(psd_blob,
>  							true)) {
>  				status = add_directory_inheritable_components(
>  							handle,
>  							smb_fname->base_name,
>  							psbuf,
> -							psd);
> +							psd_blob);
>  				if (!NT_STATUS_IS_OK(status)) {
>  					TALLOC_FREE(frame);
>  					return status;
> @@ -787,23 +787,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			   the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
>  			   can't be inherited in this way under POSIX.
>  			   Remove it for Windows-style ACLs. */
> -			psd->type &= ~SEC_DESC_DACL_PROTECTED;
> +			psd_blob->type &= ~SEC_DESC_DACL_PROTECTED;
>  		}
>  	}
>  
>  	if (!(security_info & SECINFO_OWNER)) {
> -		psd->owner_sid = NULL;
> +		psd_blob->owner_sid = NULL;
>  	}
>  	if (!(security_info & SECINFO_GROUP)) {
> -		psd->group_sid = NULL;
> +		psd_blob->group_sid = NULL;
>  	}
>  	if (!(security_info & SECINFO_DACL)) {
> -		psd->type &= ~SEC_DESC_DACL_PRESENT;
> -		psd->dacl = NULL;
> +		psd_blob->type &= ~SEC_DESC_DACL_PRESENT;
> +		psd_blob->dacl = NULL;
>  	}
>  	if (!(security_info & SECINFO_SACL)) {
> -		psd->type &= ~SEC_DESC_SACL_PRESENT;
> -		psd->sacl = NULL;
> +		psd_blob->type &= ~SEC_DESC_SACL_PRESENT;
> +		psd_blob->sacl = NULL;
>  	}
>  
>  	TALLOC_FREE(blob.data);
> @@ -811,11 +811,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	if (DEBUGLEVEL >= 10) {
>  		DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
>  			smb_fname->base_name ));
> -		NDR_PRINT_DEBUG(security_descriptor, psd);
> +		NDR_PRINT_DEBUG(security_descriptor, psd_blob);
>  	}
>  
>  	/* The VFS API is that the ACL is expected to be on mem_ctx */
> -	*ppdesc = talloc_move(mem_ctx, &psd);
> +	*ppdesc = talloc_move(mem_ctx, &psd_blob);
>  
>  	TALLOC_FREE(frame);
>  	return NT_STATUS_OK;
> -- 
> 2.7.4
> 
> 
> From b37829fdcc5dac4e289b3b6ccc5a05fad4e529a5 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 23 Aug 2016 13:11:24 +0200
> Subject: [PATCH 03/13] vfs_acl_common: rename pdesc_next to psd_fs
> 
> In most realistic cases the "next" VFS op will return the
permissions
> from the filesystem. This rename makes it explicit where the SD is
> originating from. No change in behaviour.
> 
> This just paves the way for a later change that will simplify the whole
> logic and talloc hierarchy.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 1adc875..fb01bf4 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -489,7 +489,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
>  	uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
>  	struct security_descriptor *psd_blob = NULL;
> -	struct security_descriptor *pdesc_next = NULL;
> +	struct security_descriptor *psd_fs = NULL;
>  	const struct smb_filename *smb_fname = NULL;
>  	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
>  						ACL_MODULE_NAME,
> @@ -618,13 +618,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  							  fsp,
>  							  HASH_SECURITY_INFO,
>  							  mem_ctx,
> -							  &pdesc_next);
> +							  &psd_fs);
>  		} else {
>  			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
>  							 smb_fname,
>  							 HASH_SECURITY_INFO,
>  							 mem_ctx,
> -							 &pdesc_next);
> +							 &psd_fs);
>  		}
>  
>  		if (!NT_STATUS_IS_OK(status)) {
> @@ -636,17 +636,17 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			return status;
>  		}
>  
> -		/* Ensure we don't leak psd_next if we don't choose it.
> +		/* 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, pdesc_next);
> +		talloc_steal(frame, psd_fs);
>  
> -		status = hash_sd_sha256(pdesc_next, hash_tmp);
> +		status = hash_sd_sha256(psd_fs, hash_tmp);
>  		if (!NT_STATUS_IS_OK(status)) {
>  			TALLOC_FREE(psd_blob);
> -			psd_blob = pdesc_next;
> +			psd_blob = psd_fs;
>  			goto out;
>  		}
>  
> @@ -667,11 +667,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		if (DEBUGLEVEL >= 10) {
>  			DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s
is:\n",
>  				  smb_fname->base_name ));
> -			NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
> +			NDR_PRINT_DEBUG(security_descriptor, psd_fs);
>  		}
>  
>  		TALLOC_FREE(psd_blob);
> -		psd_blob = pdesc_next;
> +		psd_blob = psd_fs;
>  	}
>    out:
>  
> @@ -684,13 +684,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  							  fsp,
>  							  security_info,
>  							  mem_ctx,
> -							  &pdesc_next);
> +							  &psd_fs);
>  		} else {
>  			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
>  							 smb_fname,
>  							 security_info,
>  							 mem_ctx,
> -							 &pdesc_next);
> +							 &psd_fs);
>  		}
>  
>  		if (!NT_STATUS_IS_OK(status)) {
> @@ -702,19 +702,19 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			return status;
>  		}
>  
> -		/* Ensure we don't leak psd_next if we don't choose it.
> +		/* 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, pdesc_next);
> -		psd_blob = pdesc_next;
> +		talloc_steal(frame, psd_fs);
> +		psd_blob = psd_fs;
>  	}
>  
> -	if (psd_blob != pdesc_next) {
> +	if (psd_blob != psd_fs) {
>  		/* We're returning the blob, throw
>   		 * away the filesystem SD. */
> -		TALLOC_FREE(pdesc_next);
> +		TALLOC_FREE(psd_fs);
>  	} else {
>  		SMB_STRUCT_STAT sbuf;
>  		SMB_STRUCT_STAT *psbuf = &sbuf;
> @@ -760,7 +760,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		is_directory = S_ISDIR(psbuf->st_ex_mode);
>  
>  		if (ignore_file_system_acl) {
> -			TALLOC_FREE(pdesc_next);
> +			TALLOC_FREE(psd_fs);
>  			status = make_default_filesystem_acl(mem_ctx,
>  						smb_fname->base_name,
>  						psbuf,
> -- 
> 2.7.4
> 
> 
> From 0d2d6afb990c382a508da1823631e1b76af57019 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 23 Aug 2016 13:14:50 +0200
> Subject: [PATCH 04/13] vfs_acl_common: remove redundant NULL assignment
> 
> The variables are already set to NULL by TALLOC_FREE.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index fb01bf4..0c40f37 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -551,7 +551,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			   (unsigned int)hash_type,
>  			   smb_fname->base_name));
>  		TALLOC_FREE(psd_blob);
> -		psd_blob = NULL;
>  		goto out;
>  	}
>  
> @@ -562,7 +561,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			   (unsigned int)hash_type,
>  			   smb_fname->base_name));
>  		TALLOC_FREE(psd_blob);
> -		psd_blob = NULL;
>  		goto out;
>  	}
>  
> -- 
> 2.7.4
> 
> 
> From 832b1f982b3800c65b2031e4f2bb51c555997cdd Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 23 Aug 2016 17:07:20 +0200
> Subject: [PATCH 05/13] 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 <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 131
++++++++++++++++++---------------------
>  1 file changed, 61 insertions(+), 70 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 0c40f37..66c58e7 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -488,6 +488,7 @@ 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;
>  	const struct smb_filename *smb_fname = NULL;
> @@ -495,7 +496,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  						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 && smb_fname_in == NULL) {
>  		smb_fname = fsp->fsp_name;
> @@ -505,11 +508,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  
>  	DEBUG(10, ("get_nt_acl_internal: name=%s\n",
smb_fname->base_name));
>  
> -	status = get_acl_blob(frame, handle, fsp, smb_fname, &blob);
> +	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &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,
> @@ -517,17 +519,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) {
> @@ -537,10 +534,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;
>  		}
>  
> @@ -569,20 +570,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,
>  						 smb_fname->base_name,
> -						 frame,
> +						 mem_ctx,
>  						 &sys_acl_blob_description,
>  						 &sys_acl_blob);
>  		}
> @@ -592,16 +591,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",
>  					   smb_fname->base_name ));
> +				psd = psd_blob;
> +				psd_blob = NULL;
>  				goto out;
>  			}
>  		}
> @@ -630,21 +633,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  				   "returned %s\n",
>  				   smb_fname->base_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;
>  		}
>  
> @@ -653,6 +650,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			DEBUG(10, ("get_nt_acl_internal: blob hash "
>  				   "matches for file %s\n",
>  				   smb_fname->base_name ));
> +			psd = psd_blob;
> +			psd_blob = NULL;
>  			goto out;
>  		}
>  
> @@ -669,11 +668,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 */
> @@ -682,13 +683,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,
>  							 smb_fname,
>  							 security_info,
>  							 mem_ctx,
> -							 &psd_fs);
> +							 &psd);
>  		}
>  
>  		if (!NT_STATUS_IS_OK(status)) {
> @@ -696,24 +697,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  				   "returned %s\n",
>  				   smb_fname->base_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;
> @@ -725,8 +715,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 {
> @@ -751,72 +740,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  						smb_fname,
>  						&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,
>  						smb_fname->base_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,
>  							smb_fname->base_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",
>  			smb_fname->base_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;
>  }
>  
>  /*********************************************************************
> -- 
> 2.7.4
> 
> 
> From 695256ac9846ee8671388c58b6ded2dbd9adbce9 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Tue, 23 Aug 2016 22:32:57 +0200
> Subject: [PATCH 06/13] vfs_acl_common: move the ACL blob validation to a
>  helper function
> 
> No change in behaviour.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 216
+++++++++++++++++++++++----------------
>  1 file changed, 127 insertions(+), 89 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 66c58e7..2bc2499 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -467,20 +467,32 @@ static NTSTATUS
make_default_filesystem_acl(TALLOC_CTX *ctx,
>  	return NT_STATUS_OK;
>  }
>  
> -/*******************************************************************
> - Pull a DATA_BLOB from an xattr given a pathname.
> - If the hash doesn't match, or doesn't exist - return the
underlying
> - filesystem sd.
> -*******************************************************************/
> -
> -static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
> -				    files_struct *fsp,
> -				    const struct smb_filename *smb_fname_in,
> -				    uint32_t security_info,
> -				    TALLOC_CTX *mem_ctx,
> -				    struct security_descriptor **ppdesc)
> +/**
> + * Validate an ACL blob
> + *
> + * This validates an ACL blob against the underlying filesystem ACL. If
this
> + * function returns NT_STATUS_OK ppsd can be
> + *
> + * 1. the ACL from the blob (psd_from_fs=false), or
> + * 2. the ACL from the fs (psd_from_fs=true), or
> + * 3. NULL (!)
> + *
> + * If the return value is anything else then NT_STATUS_OK, ppsd is set to
NULL
> + * and psd_from_fs set to false.
> + *
> + * Returning the underlying filesystem ACL in case no. 2 is really just an
> + * optimisation, because some validations have to fetch the filesytem ACL
as
> + * part of the validation, so we already have it available and callers
might
> + * need it as well.
> + **/
> +static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
> +				     vfs_handle_struct *handle,
> +				     files_struct *fsp,
> +				     const struct smb_filename *smb_fname,
> +				     DATA_BLOB *blob,
> +				     struct security_descriptor **ppsd,
> +				     bool *psd_is_from_fs)
>  {
> -	DATA_BLOB blob = data_blob_null;
>  	NTSTATUS status;
>  	uint16_t hash_type = XATTR_SD_HASH_TYPE_NONE;
>  	uint16_t xattr_version = 0;
> @@ -491,41 +503,28 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	struct security_descriptor *psd = NULL;
>  	struct security_descriptor *psd_blob = NULL;
>  	struct security_descriptor *psd_fs = NULL;
> -	const struct smb_filename *smb_fname = NULL;
> +	char *sys_acl_blob_description = NULL;
> +	DATA_BLOB sys_acl_blob = { 0 };
>  	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
>  						ACL_MODULE_NAME,
>  						"ignore system acls",
>  						false);
> -	char *sys_acl_blob_description = NULL;
> -	DATA_BLOB sys_acl_blob = { 0 };
> -	bool psd_is_from_fs = false;
>  
> -	if (fsp && smb_fname_in == NULL) {
> -		smb_fname = fsp->fsp_name;
> -	} else {
> -		smb_fname = smb_fname_in;
> -	}
> -
> -	DEBUG(10, ("get_nt_acl_internal: name=%s\n",
smb_fname->base_name));
> +	*ppsd = NULL;
> +	*psd_is_from_fs = false;
>  
> -	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
> +	status = parse_acl_blob(blob,
> +				mem_ctx,
> +				&psd_blob,
> +				&hash_type,
> +				&xattr_version,
> +				&hash[0],
> +				&sys_acl_hash[0]);
>  	if (!NT_STATUS_IS_OK(status)) {
> -		DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
> -			nt_errstr(status)));
> -		goto out;
> -	} else {
> -		status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
> -					&hash_type, &xattr_version, &hash[0],
&sys_acl_hash[0]);
> -		if (!NT_STATUS_IS_OK(status)) {
> -			DEBUG(10, ("parse_acl_blob returned %s\n",
> -				   nt_errstr(status)));
> -			TALLOC_FREE(blob.data);
> -			goto out;
> -		}
> +		DBG_DEBUG("parse_acl_blob returned %s\n", nt_errstr(status));
> +		goto fail;
>  	}
>  
> -	TALLOC_FREE(blob.data);
> -
>  	/* determine which type of xattr we got */
>  	switch (xattr_version) {
>  	case 1:
> @@ -534,35 +533,29 @@ 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;
> +		*ppsd = psd_blob;
> +		return NT_STATUS_OK;
>  	case 3:
>  	case 4:
>  		if (ignore_file_system_acl) {
> -			psd = psd_blob;
> -			psd_blob = NULL;
> -			goto out;
> +			*ppsd = psd_blob;
> +			return NT_STATUS_OK;
>  		}
>  
>  		break;
>  	default:
> -		DEBUG(10, ("get_nt_acl_internal: ACL blob revision "
> -			   "mismatch (%u) for file %s\n",
> -			   (unsigned int)hash_type,
> -			   smb_fname->base_name));
> +		DBG_DEBUG("ACL blob revision mismatch (%u) for file %s\n",
> +			  (unsigned int)hash_type, smb_fname->base_name);
>  		TALLOC_FREE(psd_blob);
> -		goto out;
> +		return NT_STATUS_OK;
>  	}
>  
>  	/* determine which type of xattr we got */
>  	if (hash_type != XATTR_SD_HASH_TYPE_SHA256) {
> -		DEBUG(10, ("get_nt_acl_internal: ACL blob hash type "
> -			   "(%u) unexpected for file %s\n",
> -			   (unsigned int)hash_type,
> -			   smb_fname->base_name));
> +		DBG_DEBUG("ACL blob hash type (%u) unexpected for file %s\n",
> +			  (unsigned int)hash_type, smb_fname->base_name);
>  		TALLOC_FREE(psd_blob);
> -		goto out;
> +		return NT_STATUS_OK;
>  	}
>  
>  	/* determine which type of xattr we got */
> @@ -600,12 +593,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  			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",
> -					   smb_fname->base_name ));
> -				psd = psd_blob;
> -				psd_blob = NULL;
> -				goto out;
> +				DBG_DEBUG("blob hash matches for file %s\n",
> +					  smb_fname->base_name);
> +				*ppsd = psd_blob;
> +				return NT_STATUS_OK;
>  			}
>  		}
>  
> @@ -629,51 +620,102 @@ static NTSTATUS
get_nt_acl_internal(vfs_handle_struct *handle,
>  		}
>  
>  		if (!NT_STATUS_IS_OK(status)) {
> -			DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
> -				   "returned %s\n",
> -				   smb_fname->base_name,
> -				   nt_errstr(status)));
> +			DBG_DEBUG("get_next_acl for file %s returned %s\n",
> +				  smb_fname->base_name, nt_errstr(status));
>  			goto fail;
>  		}
>  
>  		status = hash_sd_sha256(psd_fs, hash_tmp);
>  		if (!NT_STATUS_IS_OK(status)) {
>  			TALLOC_FREE(psd_blob);
> -			psd = psd_fs;
> -			psd_fs = NULL;
> -			psd_is_from_fs = true;
> -			goto out;
> +			*ppsd = psd_fs;
> +			*psd_is_from_fs = true;
> +			return NT_STATUS_OK;
>  		}
>  
>  		if (memcmp(&hash[0], &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",
> -				   smb_fname->base_name ));
> -			psd = psd_blob;
> -			psd_blob = NULL;
> -			goto out;
> +			DBG_DEBUG("blob hash matches for file %s\n",
> +				  smb_fname->base_name);
> +			*ppsd = psd_blob;
> +			return NT_STATUS_OK;
>  		}
>  
>  		/* Hash doesn't match, return underlying sd. */
> -		DEBUG(10, ("get_nt_acl_internal: blob hash "
> -			   "does not match for file %s - returning "
> -			   "file system SD mapping.\n",
> -			   smb_fname->base_name ));
> +		DBG_DEBUG("blob hash does not match for file %s - returning "
> +			  "file system SD mapping.\n",
> +			  smb_fname->base_name);
>  
>  		if (DEBUGLEVEL >= 10) {
> -			DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s
is:\n",
> -				  smb_fname->base_name ));
> +			DBG_DEBUG("acl for blob hash for %s is:\n",
> +				  smb_fname->base_name);
>  			NDR_PRINT_DEBUG(security_descriptor, psd_fs);
>  		}
>  
>  		TALLOC_FREE(psd_blob);
> -		psd = psd_fs;
> -		psd_fs = NULL;
> -		psd_is_from_fs = true;
> +		*ppsd = psd_fs;
> +		*psd_is_from_fs = true;
> +	}
> +
> +	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;
> +}
> +
> +/*******************************************************************
> + Pull a DATA_BLOB from an xattr given a pathname.
> + If the hash doesn't match, or doesn't exist - return the
underlying
> + filesystem sd.
> +*******************************************************************/
> +
> +static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
> +				    files_struct *fsp,
> +				    const struct smb_filename *smb_fname_in,
> +				    uint32_t security_info,
> +				    TALLOC_CTX *mem_ctx,
> +				    struct security_descriptor **ppdesc)
> +{
> +	DATA_BLOB blob = data_blob_null;
> +	NTSTATUS status;
> +	struct security_descriptor *psd = NULL;
> +	const struct smb_filename *smb_fname = NULL;
> +	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
> +						ACL_MODULE_NAME,
> +						"ignore system acls",
> +						false);
> +	bool psd_is_from_fs = false;
> +
> +	if (fsp && smb_fname_in == NULL) {
> +		smb_fname = fsp->fsp_name;
> +	} else {
> +		smb_fname = smb_fname_in;
> +	}
> +
> +	DEBUG(10, ("get_nt_acl_internal: name=%s\n",
smb_fname->base_name));
> +
> +	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
> +	if (NT_STATUS_IS_OK(status)) {
> +		status = validate_nt_acl_blob(mem_ctx,
> +					      handle,
> +					      fsp,
> +					      smb_fname,
> +					      &blob,
> +					      &psd,
> +					      &psd_is_from_fs);
> +		TALLOC_FREE(blob.data);
> +		if (!NT_STATUS_IS_OK(status)) {
> +			DBG_DEBUG("ACL validation for [%s] failed\n",
> +				  smb_fname->base_name);
> +			goto fail;
> +		}
>  	}
>  
> -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
> @@ -803,10 +845,6 @@ out:
>  
>  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;
>  }
>  
> -- 
> 2.7.4
> 
> 
> From 69cd4f376d0a287719eadf96ca3324e76317ab66 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 24 Aug 2016 10:01:17 +0200
> Subject: [PATCH 07/13] vfs_acl_tdb|xattr: use a config handle
> 
> Better for performance and a subsequent commit will add one more option
> where this will pay off.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 50
++++++++++++++++++++++++++++++++--------
>  source3/modules/vfs_acl_tdb.c    |  7 ++++++
>  source3/modules/vfs_acl_xattr.c  |  7 ++++++
>  3 files changed, 54 insertions(+), 10 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 2bc2499..fc72b98 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -46,6 +46,34 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct
*handle,
>  				SECINFO_DACL | \
>  				SECINFO_SACL)
>  
> +struct acl_common_config {
> +	bool ignore_system_acls;
> +};
> +
> +static bool init_acl_common_config(vfs_handle_struct *handle)
> +{
> +	struct acl_common_config *config = NULL;
> +
> +	config = talloc_zero(handle->conn, struct acl_common_config);
> +	if (config == NULL) {
> +		DBG_ERR("talloc_zero() failed\n");
> +		errno = ENOMEM;
> +		return false;
> +	}
> +
> +	config->ignore_system_acls = lp_parm_bool(SNUM(handle->conn),
> +						  ACL_MODULE_NAME,
> +						  "ignore system acls",
> +						  false);
> +
> +	SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
> +				struct acl_common_config,
> +				return false);
> +
> +	return true;
> +}
> +
> +
>  /*******************************************************************
>   Hash a security descriptor.
>  *******************************************************************/
> @@ -505,14 +533,15 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX
*mem_ctx,
>  	struct security_descriptor *psd_fs = NULL;
>  	char *sys_acl_blob_description = NULL;
>  	DATA_BLOB sys_acl_blob = { 0 };
> -	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
> -						ACL_MODULE_NAME,
> -						"ignore system acls",
> -						false);
> +	struct acl_common_config *config = NULL;
>  
>  	*ppsd = NULL;
>  	*psd_is_from_fs = false;
>  
> +	SMB_VFS_HANDLE_GET_DATA(handle, config,
> +				struct acl_common_config,
> +				return NT_STATUS_UNSUCCESSFUL);
> +
>  	status = parse_acl_blob(blob,
>  				mem_ctx,
>  				&psd_blob,
> @@ -537,7 +566,7 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX
*mem_ctx,
>  		return NT_STATUS_OK;
>  	case 3:
>  	case 4:
> -		if (ignore_file_system_acl) {
> +		if (config->ignore_system_acls) {
>  			*ppsd = psd_blob;
>  			return NT_STATUS_OK;
>  		}
> @@ -685,11 +714,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	NTSTATUS status;
>  	struct security_descriptor *psd = NULL;
>  	const struct smb_filename *smb_fname = NULL;
> -	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
> -						ACL_MODULE_NAME,
> -						"ignore system acls",
> -						false);
>  	bool psd_is_from_fs = false;
> +	struct acl_common_config *config = NULL;
> +
> +	SMB_VFS_HANDLE_GET_DATA(handle, config,
> +				struct acl_common_config,
> +				return NT_STATUS_UNSUCCESSFUL);
>  
>  	if (fsp && smb_fname_in == NULL) {
>  		smb_fname = fsp->fsp_name;
> @@ -788,7 +818,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		}
>  		is_directory = S_ISDIR(psbuf->st_ex_mode);
>  
> -		if (ignore_file_system_acl) {
> +		if (config->ignore_system_acls) {
>  			TALLOC_FREE(psd);
>  			status = make_default_filesystem_acl(mem_ctx,
>  						smb_fname->base_name,
> diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
> index e4c8462..0c92b72 100644
> --- a/source3/modules/vfs_acl_tdb.c
> +++ b/source3/modules/vfs_acl_tdb.c
> @@ -308,6 +308,7 @@ static int connect_acl_tdb(struct vfs_handle_struct
*handle,
>  				const char *user)
>  {
>  	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
> +	bool ok;
>  
>  	if (ret < 0) {
>  		return ret;
> @@ -318,6 +319,12 @@ static int connect_acl_tdb(struct vfs_handle_struct
*handle,
>  		return -1;
>  	}
>  
> +	ok = init_acl_common_config(handle);
> +	if (!ok) {
> +		DBG_ERR("init_acl_common_config failed\n");
> +		return -1;
> +	}
> +
>  	/* Ensure we have the parameters correct if we're
>  	 * using this module. */
>  	DEBUG(2,("connect_acl_tdb: setting 'inherit acls = true'
"
> diff --git a/source3/modules/vfs_acl_xattr.c
b/source3/modules/vfs_acl_xattr.c
> index d311c57..307ab6a 100644
> --- a/source3/modules/vfs_acl_xattr.c
> +++ b/source3/modules/vfs_acl_xattr.c
> @@ -180,11 +180,18 @@ static int connect_acl_xattr(struct vfs_handle_struct
*handle,
>  				const char *user)
>  {
>  	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
> +	bool ok;
>  
>  	if (ret < 0) {
>  		return ret;
>  	}
>  
> +	ok = init_acl_common_config(handle);
> +	if (!ok) {
> +		DBG_ERR("init_acl_common_config failed\n");
> +		return -1;
> +	}
> +
>  	/* Ensure we have the parameters correct if we're
>  	 * using this module. */
>  	DEBUG(2,("connect_acl_xattr: setting 'inherit acls = true'
"
> -- 
> 2.7.4
> 
> 
> From cdb2da35e9ddc394b9b4fef3080de4a1cae35000 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 24 Aug 2016 10:30:15 +0200
> Subject: [PATCH 08/13] vfs_acl_common: move stat stuff to a helper function
> 
> Will be reused in the next commit when moving the
> make_default_filesystem_acl() stuff to a different place.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 79
++++++++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index fc72b98..cf50689 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -697,6 +697,48 @@ fail:
>  	return status;
>  }
>  
> +static NTSTATUS stat_fsp_or_smb_fname(vfs_handle_struct *handle,
> +				      files_struct *fsp,
> +				      const struct smb_filename *smb_fname,
> +				      SMB_STRUCT_STAT *sbuf,
> +				      SMB_STRUCT_STAT **psbuf)
> +{
> +	NTSTATUS status;
> +	int ret;
> +
> +	if (fsp) {
> +		status = vfs_stat_fsp(fsp);
> +		if (!NT_STATUS_IS_OK(status)) {
> +			return status;
> +		}
> +		*psbuf = &fsp->fsp_name->st;
> +	} else {
> +		/*
> +		 * https://bugzilla.samba.org/show_bug.cgi?id=11249
> +		 *
> +		 * We are currently guaranteed that 'name' here is a
> +		 * smb_fname->base_name, which *cannot* contain a stream name
> +		 * (':'). vfs_stat_smb_fname() splits a name into a base name +
> +		 * stream name, which when we get here we know we've already
> +		 * done.  So we have to call the stat or lstat VFS calls
> +		 * directly here. Else, a base_name that contains a ':' (from a
> +		 * demangled name) will get split again.
> +		 *
> +		 * FIXME.
> +		 * This uglyness will go away once smb_fname is fully plumbed
> +		 * through the VFS.
> +		 */
> +		ret = vfs_stat_smb_basename(handle->conn,
> +					    smb_fname,
> +					    sbuf);
> +		if (ret == -1) {
> +			return map_nt_error_from_unix(errno);
> +		}
> +	}
> +
> +	return NT_STATUS_OK;
> +}
> +
>  /*******************************************************************
>   Pull a DATA_BLOB from an xattr given a pathname.
>   If the hash doesn't match, or doesn't exist - return the
underlying
> @@ -784,38 +826,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		 * filesystem. If it's a directory, and has no
>  		 * inheritable ACE entries we have to fake them.
>  		 */
> -		if (fsp) {
> -			status = vfs_stat_fsp(fsp);
> -			if (!NT_STATUS_IS_OK(status)) {
> -				goto fail;
> -			}
> -			psbuf = &fsp->fsp_name->st;
> -		} else {
> -			/*
> -			 * https://bugzilla.samba.org/show_bug.cgi?id=11249
> -			 *
> -			 * We are currently guaranteed that 'name' here is
> -			 * a smb_fname->base_name, which *cannot* contain
> -			 * a stream name (':'). vfs_stat_smb_fname() splits
> -			 * a name into a base name + stream name, which
> -			 * when we get here we know we've already done.
> -			 * So we have to call the stat or lstat VFS
> -			 * calls directly here. Else, a base_name that
> -			 * contains a ':' (from a demangled name) will
> -			 * get split again.
> -			 *
> -			 * FIXME.
> -			 * This uglyness will go away once smb_fname
> -			 * is fully plumbed through the VFS.
> -			 */
> -			int ret = vfs_stat_smb_basename(handle->conn,
> -						smb_fname,
> -						&sbuf);
> -			if (ret == -1) {
> -				status = map_nt_error_from_unix(errno);
> -				goto fail;
> -			}
> +
> +		status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
> +					       &sbuf, &psbuf);
> +		if (!NT_STATUS_IS_OK(status)) {
> +			goto fail;
>  		}
> +
>  		is_directory = S_ISDIR(psbuf->st_ex_mode);
>  
>  		if (config->ignore_system_acls) {
> -- 
> 2.7.4
> 
> 
> From 7c9c463cdf2204ab7d4cdb7c8bfb8164765615fa Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 24 Aug 2016 10:43:47 +0200
> Subject: [PATCH 09/13] vfs_acl_common: check for ignore_system_acls before
>  fetching filesystem ACL
> 
> If ignore_system_acls is set and we're synthesizing a default ACL, we
> were fetching the filesystem ACL just to free it again. This change
> avoids this.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 99
++++++++++++++++++++++------------------
>  1 file changed, 55 insertions(+), 44 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index cf50689..93b4e39 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -792,35 +792,57 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		/* Get the full underlying sd, as we failed to get the
>  		 * blob for the hash, or the revision/hash type wasn't
>  		 * known */
> -		if (fsp) {
> -			status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
> -							  fsp,
> -							  security_info,
> -							  mem_ctx,
> -							  &psd);
> +
> +		if (config->ignore_system_acls) {
> +			SMB_STRUCT_STAT sbuf;
> +			SMB_STRUCT_STAT *psbuf = &sbuf;
> +
> +			status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
> +						       &sbuf, &psbuf);
> +			if (!NT_STATUS_IS_OK(status)) {
> +				goto fail;
> +			}
> +
> +			status = make_default_filesystem_acl(
> +				mem_ctx,
> +				smb_fname->base_name,
> +				psbuf,
> +				&psd);
> +			if (!NT_STATUS_IS_OK(status)) {
> +				goto fail;
> +			}
>  		} else {
> -			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
> -							 smb_fname,
> -							 security_info,
> -							 mem_ctx,
> -							 &psd);
> -		}
> +			if (fsp) {
> +				status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
> +								  fsp,
> +								  security_info,
> +								  mem_ctx,
> +								  &psd);
> +			} else {
> +				status = SMB_VFS_NEXT_GET_NT_ACL(handle,
> +								 smb_fname,
> +								 security_info,
> +								 mem_ctx,
> +								 &psd);
> +			}
>  
> -		if (!NT_STATUS_IS_OK(status)) {
> -			DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
> -				   "returned %s\n",
> -				   smb_fname->base_name,
> -				   nt_errstr(status)));
> -			goto fail;
> -		}
> +			if (!NT_STATUS_IS_OK(status)) {
> +				DBG_DEBUG("get_next_acl for file %s "
> +					  "returned %s\n",
> +					  smb_fname->base_name,
> +					  nt_errstr(status));
> +				goto fail;
> +			}
>  
> -		psd_is_from_fs = true;
> +			psd_is_from_fs = true;
> +		}
>  	}
>  
>  	if (psd_is_from_fs) {
>  		SMB_STRUCT_STAT sbuf;
>  		SMB_STRUCT_STAT *psbuf = &sbuf;
>  		bool is_directory = false;
> +
>  		/*
>  		 * We're returning the underlying ACL from the
>  		 * filesystem. If it's a directory, and has no
> @@ -835,34 +857,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  
>  		is_directory = S_ISDIR(psbuf->st_ex_mode);
>  
> -		if (config->ignore_system_acls) {
> -			TALLOC_FREE(psd);
> -			status = make_default_filesystem_acl(mem_ctx,
> -						smb_fname->base_name,
> -						psbuf,
> -						&psd);
> +		if (is_directory && !sd_has_inheritable_components(psd, true)) {
> +			status = add_directory_inheritable_components(
> +				handle,
> +				smb_fname->base_name,
> +				psbuf,
> +				psd);
>  			if (!NT_STATUS_IS_OK(status)) {
>  				goto fail;
>  			}
> -		} else {
> -			if (is_directory &&
> -				!sd_has_inheritable_components(psd,
> -							true)) {
> -				status = add_directory_inheritable_components(
> -							handle,
> -							smb_fname->base_name,
> -							psbuf,
> -							psd);
> -				if (!NT_STATUS_IS_OK(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->type &= ~SEC_DESC_DACL_PROTECTED;
>  		}
> +
> +		/*
> +		 * 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->type &= ~SEC_DESC_DACL_PROTECTED;
>  	}
>  
>  	if (!(security_info & SECINFO_OWNER)) {
> -- 
> 2.7.4
> 
> 
> From ec7c255a37b72df31f859343eaeb07065aa4653c Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 24 Aug 2016 20:31:00 +0200
> Subject: [PATCH 10/13] vfs_acl_xattr|tdb: add option to control default ACL
>  style
> 
> Existing behaviour is "posix" style. Next commit will (re)add the
> "windows" style. This commit doesn't change behaviour in any
way.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  docs-xml/manpages/vfs_acl_tdb.8.xml   | 25 ++++++++++++++++++
>  docs-xml/manpages/vfs_acl_xattr.8.xml | 25 ++++++++++++++++++
>  source3/modules/vfs_acl_common.c      | 48
++++++++++++++++++++++++++++++-----
>  3 files changed, 92 insertions(+), 6 deletions(-)
> 
> diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml
b/docs-xml/manpages/vfs_acl_tdb.8.xml
> index 640cec0..607e344 100644
> --- a/docs-xml/manpages/vfs_acl_tdb.8.xml
> +++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
> @@ -63,6 +63,31 @@
>  		</para>
>  		</listitem>
>  		</varlistentry>
> +
> +		<varlistentry>
> +		<term>acl_tdb:default acl style = [posix|windows]</term>
> +		<listitem>
> +		<para>
> +		This parameter determines the type of ACL that is synthesized in
> +		case a file or directory lacks an
> +		<emphasis>security.NTACL</emphasis> xattr.
> +		</para>
> +		<para>
> +		When set to <emphasis>posix</emphasis>, an ACL will be
> +		synthesized based on the POSIX mode permissions for user, group
> +		and others, with an additional ACE for <emphasis>NT
> +		Authority\SYSTEM</emphasis> will full rights.
> +		</para>
> +		<para>
> +		When set to <emphasis>windows</emphasis>, an ACL is
synthesized
> +		the same way Windows does it, only including permissions for the
> +		owner and <emphasis>NT Authority\SYSTEM</emphasis>.
> +		</para>
> +		<para>
> +		The default for this option is <emphasis>posix</emphasis>.
> +		</para>
> +		</listitem>
> +		</varlistentry>
>  	</variablelist>
>  
>  </refsect1>
> diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml
b/docs-xml/manpages/vfs_acl_xattr.8.xml
> index 60a1c2d..8da73e0 100644
> --- a/docs-xml/manpages/vfs_acl_xattr.8.xml
> +++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
> @@ -67,6 +67,31 @@
>  		</para>
>  		</listitem>
>  		</varlistentry>
> +
> +		<varlistentry>
> +		<term>acl_xattr:default acl style = [posix|windows]</term>
> +		<listitem>
> +		<para>
> +		This parameter determines the type of ACL that is synthesized in
> +		case a file or directory lacks an
> +		<emphasis>security.NTACL</emphasis> xattr.
> +		</para>
> +		<para>
> +		When set to <emphasis>posix</emphasis>, an ACL will be
> +		synthesized based on the POSIX mode permissions for user, group
> +		and others, with an additional ACE for <emphasis>NT
> +		Authority\SYSTEM</emphasis> will full rights.
> +		</para>
> +		<para>
> +		When set to <emphasis>windows</emphasis>, an ACL is
synthesized
> +		the same way Windows does it, only including permissions for the
> +		owner and <emphasis>NT Authority\SYSTEM</emphasis>.
> +		</para>
> +		<para>
> +		The default for this option is <emphasis>posix</emphasis>.
> +		</para>
> +		</listitem>
> +		</varlistentry>
>  	</variablelist>
>  
>  </refsect1>
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 93b4e39..45ce532 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -46,8 +46,16 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct
*handle,
>  				SECINFO_DACL | \
>  				SECINFO_SACL)
>  
> +enum default_acl_style {DEFAULT_ACL_POSIX, DEFAULT_ACL_WINDOWS};
> +
> +static const struct enum_list default_acl_style[] = {
> +	{DEFAULT_ACL_POSIX,	"posix"},
> +	{DEFAULT_ACL_WINDOWS,	"windows"}
> +};
> +
>  struct acl_common_config {
>  	bool ignore_system_acls;
> +	enum default_acl_style default_acl_style;
>  };
>  
>  static bool init_acl_common_config(vfs_handle_struct *handle)
> @@ -65,6 +73,11 @@ static bool init_acl_common_config(vfs_handle_struct
*handle)
>  						  ACL_MODULE_NAME,
>  						  "ignore system acls",
>  						  false);
> +	config->default_acl_style = lp_parm_enum(SNUM(handle->conn),
> +						 ACL_MODULE_NAME,
> +						 "default acl style",
> +						 default_acl_style,
> +						 DEFAULT_ACL_POSIX);
>  
>  	SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
>  				struct acl_common_config,
> @@ -387,10 +400,10 @@ static NTSTATUS
add_directory_inheritable_components(vfs_handle_struct *handle,
>  	return NT_STATUS_OK;
>  }
>  
> -static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
> -					    const char *name,
> -					    SMB_STRUCT_STAT *psbuf,
> -					    struct security_descriptor **ppdesc)
> +static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx,
> +				       const char *name,
> +				       SMB_STRUCT_STAT *psbuf,
> +				       struct security_descriptor **ppdesc)
>  {
>  	struct dom_sid owner_sid, group_sid;
>  	size_t size = 0;
> @@ -400,8 +413,7 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX
*ctx,
>  	struct security_acl *new_dacl = NULL;
>  	int idx = 0;
>  
> -	DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n",
> -		name, (int)mode ));
> +	DBG_DEBUG("file %s mode = 0%o\n",name, (int)mode);
>  
>  	uid_to_sid(&owner_sid, psbuf->st_ex_uid);
>  	gid_to_sid(&group_sid, psbuf->st_ex_gid);
> @@ -495,6 +507,29 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX
*ctx,
>  	return NT_STATUS_OK;
>  }
>  
> +static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
> +					    struct acl_common_config *config,
> +					    const char *name,
> +					    SMB_STRUCT_STAT *psbuf,
> +					    struct security_descriptor **ppdesc)
> +{
> +	NTSTATUS status;
> +
> +	switch (config->default_acl_style) {
> +
> +	case DEFAULT_ACL_POSIX:
> +		status =  make_default_acl_posix(ctx, name, psbuf, ppdesc);
> +		break;
> +
> +	default:
> +		DBG_ERR("unknown acl style %d", config->default_acl_style);
> +		status = NT_STATUS_INTERNAL_ERROR;
> +		break;
> +	}
> +
> +	return status;
> +}
> +
>  /**
>   * Validate an ACL blob
>   *
> @@ -805,6 +840,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  
>  			status = make_default_filesystem_acl(
>  				mem_ctx,
> +				config,
>  				smb_fname->base_name,
>  				psbuf,
>  				&psd);
> -- 
> 2.7.4
> 
> 
> From 7afcd4f2164b83a3aaf3b651b69e4a5ac03b8d24 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 25 Aug 2016 07:45:34 +0200
> Subject: [PATCH 11/13] vfs_acl_common: Windows style default ACL
> 
> Reintroduce Windows style default ACL, but this time as an optional
> feature, not changing default behaviour.
> 
> Original bugreport that got reverted because it changed the default
> behaviour: https://bugzilla.samba.org/show_bug.cgi?id=12028
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 76
++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 45ce532..9a5b9bd 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -507,6 +507,78 @@ static NTSTATUS make_default_acl_posix(TALLOC_CTX
*ctx,
>  	return NT_STATUS_OK;
>  }
>  
> +static NTSTATUS make_default_acl_windows(TALLOC_CTX *ctx,
> +					 const char *name,
> +					 SMB_STRUCT_STAT *psbuf,
> +					 struct security_descriptor **ppdesc)
> +{
> +	struct dom_sid owner_sid, group_sid;
> +	size_t size = 0;
> +	struct security_ace aces[4];
> +	uint32_t access_mask = 0;
> +	mode_t mode = psbuf->st_ex_mode;
> +	struct security_acl *new_dacl = NULL;
> +	int idx = 0;
> +
> +	DBG_DEBUG("file [%s] mode [0%o]\n", name, (int)mode);
> +
> +	uid_to_sid(&owner_sid, psbuf->st_ex_uid);
> +	gid_to_sid(&group_sid, psbuf->st_ex_gid);
> +
> +	/*
> +	 * We provide 2 ACEs:
> +	 * - Owner
> +	 * - NT System
> +	 */
> +
> +	if (mode & S_IRUSR) {
> +		if (mode & S_IWUSR) {
> +			access_mask |= SEC_RIGHTS_FILE_ALL;
> +		} else {
> +			access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
> +		}
> +	}
> +	if (mode & S_IWUSR) {
> +		access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
> +	}
> +
> +	init_sec_ace(&aces[idx],
> +		     &owner_sid,
> +		     SEC_ACE_TYPE_ACCESS_ALLOWED,
> +		     access_mask,
> +		     0);
> +	idx++;
> +
> +	init_sec_ace(&aces[idx],
> +		     &global_sid_System,
> +		     SEC_ACE_TYPE_ACCESS_ALLOWED,
> +		     SEC_RIGHTS_FILE_ALL,
> +		     0);
> +	idx++;
> +
> +	new_dacl = make_sec_acl(ctx,
> +				NT4_ACL_REVISION,
> +				idx,
> +				aces);
> +
> +	if (!new_dacl) {
> +		return NT_STATUS_NO_MEMORY;
> +	}
> +
> +	*ppdesc = make_sec_desc(ctx,
> +				SECURITY_DESCRIPTOR_REVISION_1,
> +				SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT,
> +				&owner_sid,
> +				&group_sid,
> +				NULL,
> +				new_dacl,
> +				&size);
> +	if (!*ppdesc) {
> +		return NT_STATUS_NO_MEMORY;
> +	}
> +	return NT_STATUS_OK;
> +}
> +
>  static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
>  					    struct acl_common_config *config,
>  					    const char *name,
> @@ -521,6 +593,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX
*ctx,
>  		status =  make_default_acl_posix(ctx, name, psbuf, ppdesc);
>  		break;
>  
> +	case DEFAULT_ACL_WINDOWS:
> +		status =  make_default_acl_windows(ctx, name, psbuf, ppdesc);
> +		break;
> +
>  	default:
>  		DBG_ERR("unknown acl style %d", config->default_acl_style);
>  		status = NT_STATUS_INTERNAL_ERROR;
> -- 
> 2.7.4
> 
> 
> From cb873d7d6707d55c5d4a2cc5438d09c1d979c53e Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Thu, 25 Aug 2016 16:30:24 +0200
> Subject: [PATCH 12/13] s4/torture: tests for vfs_acl_xattr default ACL
styles
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  selftest/target/Samba3.pm       |   8 +
>  source3/selftest/tests.py       |   4 +-
>  source4/torture/vfs/acl_xattr.c | 314
++++++++++++++++++++++++++++++++++++++++
>  source4/torture/vfs/vfs.c       |   1 +
>  source4/torture/wscript_build   |   2 +-
>  5 files changed, 327 insertions(+), 2 deletions(-)
>  create mode 100644 source4/torture/vfs/acl_xattr.c
> 
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index 8fc3204..f68d7de 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -1792,6 +1792,14 @@ sub provision($$$$$$$$)
>  	vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
>  	inherit owner = yes
>  	include = $dfqconffile
> +[acl_xattr_ign_sysacl_posix]
> +	copy = tmp
> +	acl_xattr:ignore system acls = yes
> +	acl_xattr:default acl style = posix
> +[acl_xattr_ign_sysacl_windows]
> +	copy = tmp
> +	acl_xattr:ignore system acls = yes
> +	acl_xattr:default acl style = windows
>  	";
>  	close(CONF);
>  
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 7d0dcc1..e4c31c6 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -322,7 +322,7 @@ nbt = ["nbt.dgram" ]
>  
>  libsmbclient = ["libsmbclient"]
>  
> -vfs = ["vfs.fruit"]
> +vfs = ["vfs.fruit", "vfs.acl_xattr"]
>  
>  tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient +
idmap + vfs
>  
> @@ -418,6 +418,8 @@ for t in tests:
>          plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp
-U$USERNAME%$PASSWORD --signing=required')
>      elif t == "smb2.dosmode":
>          plansmbtorture4testsuite(t, "simpleserver",
'//$SERVER/dosmode -U$USERNAME%$PASSWORD')
> +    elif t == "vfs.acl_xattr":
> +        plansmbtorture4testsuite(t, "nt4_dc",
'//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>      else:
>          plansmbtorture4testsuite(t, "nt4_dc",
'//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>          plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp
-U$USERNAME%$PASSWORD')
> diff --git a/source4/torture/vfs/acl_xattr.c
b/source4/torture/vfs/acl_xattr.c
> new file mode 100644
> index 0000000..7fd10d0
> --- /dev/null
> +++ b/source4/torture/vfs/acl_xattr.c
> @@ -0,0 +1,314 @@
> +/*
> +   Unix SMB/CIFS implementation.
> +
> +   Copyright (C) Ralph Boehme 2016
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see
<http://www.gnu.org/licenses/>.
> +*/
> +
> +#include "includes.h"
> +#include "lib/cmdline/popt_common.h"
> +#include "libcli/smb2/smb2.h"
> +#include "libcli/smb2/smb2_calls.h"
> +#include "libcli/smb/smbXcli_base.h"
> +#include "torture/torture.h"
> +#include "torture/vfs/proto.h"
> +#include "libcli/resolve/resolve.h"
> +#include "torture/util.h"
> +#include "torture/smb2/proto.h"
> +#include "libcli/security/security.h"
> +#include "librpc/gen_ndr/ndr_security.h"
> +#include "lib/param/param.h"
> +
> +#define BASEDIR "smb2-testsd"
> +
> +#define CHECK_SECURITY_DESCRIPTOR(_sd1, _sd2) do { \
> +	if (!security_descriptor_equal(_sd1, _sd2)) { \
> +		torture_warning(tctx, "security descriptors don't
match!\n"); \
> +		torture_warning(tctx, "got:\n"); \
> +		NDR_PRINT_DEBUG(security_descriptor, _sd1); \
> +		torture_warning(tctx, "expected:\n"); \
> +		NDR_PRINT_DEBUG(security_descriptor, _sd2); \
> +		torture_result(tctx, TORTURE_FAIL, \
> +			       "%s: security descriptors don't match!\n", \
> +			       __location__); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
> +/**
> + * SMB2 connect with explicit share
> + **/
> +static bool torture_smb2_con_share(struct torture_context *tctx,
> +                           const char *share,
> +                           struct smb2_tree **tree)
> +{
> +        struct smbcli_options options;
> +        NTSTATUS status;
> +        const char *host = torture_setting_string(tctx, "host",
NULL);
> +        struct cli_credentials *credentials = cmdline_credentials;
> +
> +        lpcfg_smbcli_options(tctx->lp_ctx, &options);
> +
> +        status = smb2_connect_ext(tctx,
> +                                  host,
> +                                  lpcfg_smb_ports(tctx->lp_ctx),
> +                                  share,
> +                                  lpcfg_resolve_context(tctx->lp_ctx),
> +                                  credentials,
> +                                  0,
> +                                  tree,
> +                                  tctx->ev,
> +                                  &options,
> +                                  lpcfg_socket_options(tctx->lp_ctx),
> +                                  lpcfg_gensec_settings(tctx,
tctx->lp_ctx)
> +                                  );
> +        if (!NT_STATUS_IS_OK(status)) {
> +                printf("Failed to connect to SMB2 share \\\\%s\\%s -
%s\n",
> +                       host, share, nt_errstr(status));
> +                return false;
> +        }
> +        return true;
> +}
> +
> +static bool test_default_acl_posix(struct torture_context *tctx,
> +				   struct smb2_tree *tree_unused)
> +{
> +	struct smb2_tree *tree = NULL;
> +	NTSTATUS status;
> +	bool ok;
> +	bool ret = true;
> +	const char *dname = BASEDIR "\\testdir";
> +	const char *fname = BASEDIR "\\testdir\\testfile";
> +	struct smb2_handle fhandle, dhandle;
> +	union smb_fileinfo q;
> +	union smb_setfileinfo set;
> +	struct security_descriptor *sd = NULL;
> +	struct security_descriptor *exp_sd = NULL;
> +	char *owner_sid = NULL;
> +	char *group_sid = NULL;
> +
> +	ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_posix",
&tree);
> +	torture_assert_goto(tctx, ok == true, ret, done,
> +			    "Unable to connect to
'acl_xattr_ign_sysacl_posix'\n");
> +
> +	ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
> +	torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup
testdir\n");
> +
> +	ZERO_STRUCT(dhandle);
> +	status = torture_smb2_testdir(tree, dname, &dhandle);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"torture_smb2_testdir\n");
> +
> +	torture_comment(tctx, "Get the original sd\n");
> +
> +	ZERO_STRUCT(q);
> +	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
> +	q.query_secdesc.in.file.handle = dhandle;
> +	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER |
SECINFO_GROUP;
> +	status = smb2_getinfo_file(tree, tctx, &q);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_getinfo_file\n");
> +
> +	sd = q.query_secdesc.out.sd;
> +	owner_sid = dom_sid_string(tctx, sd->owner_sid);
> +	group_sid = dom_sid_string(tctx, sd->group_sid);
> +	torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid,
group_sid);
> +
> +	torture_comment(tctx, "Set ACL with no inheritable ACE\n");
> +
> +	sd = security_descriptor_dacl_create(tctx,
> +					     0, NULL, NULL,
> +					     owner_sid,
> +					     SEC_ACE_TYPE_ACCESS_ALLOWED,
> +					     SEC_RIGHTS_DIR_ALL,
> +					     0,
> +					     NULL);
> +
> +	ZERO_STRUCT(set);
> +	set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
> +	set.set_secdesc.in.file.handle = dhandle;
> +	set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
> +	set.set_secdesc.in.sd = sd;
> +	status = smb2_setinfo_file(tree, &set);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_setinfo_file\n");
> +
> +	TALLOC_FREE(sd);
> +	smb2_util_close(tree, dhandle);
> +
> +	torture_comment(tctx, "Create file\n");
> +
> +	ZERO_STRUCT(fhandle);
> +	status = torture_smb2_testfile(tree, fname, &fhandle);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_create_complex_file\n");
> +
> +	torture_comment(tctx, "Query file SD\n");
> +
> +	ZERO_STRUCT(q);
> +	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
> +	q.query_secdesc.in.file.handle = fhandle;
> +	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER |
SECINFO_GROUP;
> +	status = smb2_getinfo_file(tree, tctx, &q);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_getinfo_file\n");
> +	sd = q.query_secdesc.out.sd;
> +
> +	smb2_util_close(tree, fhandle);
> +	ZERO_STRUCT(fhandle);
> +
> +	torture_comment(tctx, "Checking actual file SD against expected
SD\n");
> +
> +	exp_sd = security_descriptor_dacl_create(
> +		tctx, 0, owner_sid, group_sid,
> +		owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
> +		group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ |
SEC_FILE_EXECUTE, 0,
> +		SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ |
SEC_FILE_EXECUTE, 0,
> +		SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
> +		NULL);
> +
> +	CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
> +
> +done:
> +	if (!smb2_util_handle_empty(fhandle)) {
> +		smb2_util_close(tree, fhandle);
> +	}
> +	if (!smb2_util_handle_empty(dhandle)) {
> +		smb2_util_close(tree, dhandle);
> +	}
> +	if (tree != NULL) {
> +		smb2_deltree(tree, BASEDIR);
> +		smb2_tdis(tree);
> +	}
> +
> +	return ret;
> +}
> +
> +static bool test_default_acl_win(struct torture_context *tctx,
> +				   struct smb2_tree *tree_unused)
> +{
> +	struct smb2_tree *tree = NULL;
> +	NTSTATUS status;
> +	bool ok;
> +	bool ret = true;
> +	const char *dname = BASEDIR "\\testdir";
> +	const char *fname = BASEDIR "\\testdir\\testfile";
> +	struct smb2_handle fhandle, dhandle;
> +	union smb_fileinfo q;
> +	union smb_setfileinfo set;
> +	struct security_descriptor *sd = NULL;
> +	struct security_descriptor *exp_sd = NULL;
> +	char *owner_sid = NULL;
> +	char *group_sid = NULL;
> +
> +	ok = torture_smb2_con_share(tctx,
"acl_xattr_ign_sysacl_windows", &tree);
> +	torture_assert_goto(tctx, ok == true, ret, done,
> +			    "Unable to connect to
'acl_xattr_ign_sysacl_windows'\n");
> +
> +	ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
> +	torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup
testdir\n");
> +
> +	ZERO_STRUCT(dhandle);
> +	status = torture_smb2_testdir(tree, dname, &dhandle);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"torture_smb2_testdir\n");
> +
> +	torture_comment(tctx, "Get the original sd\n");
> +
> +	ZERO_STRUCT(q);
> +	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
> +	q.query_secdesc.in.file.handle = dhandle;
> +	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER |
SECINFO_GROUP;
> +	status = smb2_getinfo_file(tree, tctx, &q);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_getinfo_file\n");
> +
> +	sd = q.query_secdesc.out.sd;
> +	owner_sid = dom_sid_string(tctx, sd->owner_sid);
> +	group_sid = dom_sid_string(tctx, sd->group_sid);
> +	torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid,
group_sid);
> +
> +	torture_comment(tctx, "Set ACL with no inheritable ACE\n");
> +
> +	sd = security_descriptor_dacl_create(tctx,
> +					     0, NULL, NULL,
> +					     owner_sid,
> +					     SEC_ACE_TYPE_ACCESS_ALLOWED,
> +					     SEC_RIGHTS_DIR_ALL,
> +					     0,
> +					     NULL);
> +
> +	ZERO_STRUCT(set);
> +	set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
> +	set.set_secdesc.in.file.handle = dhandle;
> +	set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
> +	set.set_secdesc.in.sd = sd;
> +	status = smb2_setinfo_file(tree, &set);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_setinfo_file\n");
> +
> +	TALLOC_FREE(sd);
> +	smb2_util_close(tree, dhandle);
> +
> +	torture_comment(tctx, "Create file\n");
> +
> +	ZERO_STRUCT(fhandle);
> +	status = torture_smb2_testfile(tree, fname, &fhandle);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_create_complex_file\n");
> +
> +	torture_comment(tctx, "Query file SD\n");
> +
> +	ZERO_STRUCT(q);
> +	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
> +	q.query_secdesc.in.file.handle = fhandle;
> +	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER |
SECINFO_GROUP;
> +	status = smb2_getinfo_file(tree, tctx, &q);
> +	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
"smb2_getinfo_file\n");
> +	sd = q.query_secdesc.out.sd;
> +
> +	smb2_util_close(tree, fhandle);
> +	ZERO_STRUCT(fhandle);
> +
> +	torture_comment(tctx, "Checking actual file SD against expected
SD\n");
> +
> +	exp_sd = security_descriptor_dacl_create(
> +		tctx, 0, owner_sid, group_sid,
> +		owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
> +		SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
> +		NULL);
> +
> +	CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
> +
> +done:
> +	if (!smb2_util_handle_empty(fhandle)) {
> +		smb2_util_close(tree, fhandle);
> +	}
> +	if (!smb2_util_handle_empty(dhandle)) {
> +		smb2_util_close(tree, dhandle);
> +	}
> +	if (tree != NULL) {
> +		smb2_deltree(tree, BASEDIR);
> +		smb2_tdis(tree);
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> +   basic testing of vfs_acl_xattr
> +*/
> +struct torture_suite *torture_acl_xattr(void)
> +{
> +	struct torture_suite *suite =
torture_suite_create(talloc_autofree_context(), "acl_xattr");
> +
> +	torture_suite_add_1smb2_test(suite, "default-acl-style-posix",
test_default_acl_posix);
> +	torture_suite_add_1smb2_test(suite,
"default-acl-style-windows", test_default_acl_win);
> +
> +	suite->description = talloc_strdup(suite, "vfs_acl_xattr
tests");
> +
> +	return suite;
> +}
> diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c
> index f3ce447..7f805f4 100644
> --- a/source4/torture/vfs/vfs.c
> +++ b/source4/torture/vfs/vfs.c
> @@ -107,6 +107,7 @@ NTSTATUS torture_vfs_init(void)
>  	suite->description = talloc_strdup(suite, "VFS modules
tests");
>  
>  	torture_suite_add_suite(suite, torture_vfs_fruit());
> +	torture_suite_add_suite(suite, torture_acl_xattr());
>  
>  	torture_register_suite(suite);
>  
> diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build
> index 382e2c6..ff79c3d 100755
> --- a/source4/torture/wscript_build
> +++ b/source4/torture/wscript_build
> @@ -271,7 +271,7 @@ bld.SAMBA_MODULE('TORTURE_NTP',
>  	)
>  
>  bld.SAMBA_MODULE('TORTURE_VFS',
> -	source='vfs/vfs.c vfs/fruit.c',
> +	source='vfs/vfs.c vfs/fruit.c vfs/acl_xattr.c',
>  	subsystem='smbtorture',
>  	deps='LIBCLI_SMB POPT_CREDENTIALS TORTURE_UTIL smbclient-raw
TORTURE_RAW',
>  	internal_module=True,
> -- 
> 2.7.4
> 
> 
> From 4b5d7a5577094544aa52b65a22cc0c61fde97478 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sat, 27 Aug 2016 10:11:14 +0200
> Subject: [PATCH 13/13] vfs_acl_common: use DBG_LEVEL and remove function
>  prefixes in DEBUG statements
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/modules/vfs_acl_common.c | 75
++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 42 deletions(-)
> 
> diff --git a/source3/modules/vfs_acl_common.c
b/source3/modules/vfs_acl_common.c
> index 9a5b9bd..2163a75 100644
> --- a/source3/modules/vfs_acl_common.c
> +++ b/source3/modules/vfs_acl_common.c
> @@ -144,8 +144,8 @@ static NTSTATUS parse_acl_blob(const DATA_BLOB *pblob,
>  			(ndr_pull_flags_fn_t)ndr_pull_xattr_NTACL);
>  
>  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
> -		DEBUG(5, ("parse_acl_blob: ndr_pull_xattr_NTACL failed: %s\n",
> -			ndr_errstr(ndr_err)));
> +		DBG_INFO("ndr_pull_xattr_NTACL failed: %s\n",
> +			 ndr_errstr(ndr_err));
>  		TALLOC_FREE(frame);
>  		return ndr_map_error2ntstatus(ndr_err);
>  	}
> @@ -241,8 +241,8 @@ static NTSTATUS create_acl_blob(const struct
security_descriptor *psd,
>  			(ndr_push_flags_fn_t)ndr_push_xattr_NTACL);
>  
>  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
> -		DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed:
%s\n",
> -			ndr_errstr(ndr_err)));
> +		DBG_INFO("ndr_push_xattr_NTACL failed: %s\n",
> +			 ndr_errstr(ndr_err));
>  		return ndr_map_error2ntstatus(ndr_err);
>  	}
>  
> @@ -287,8 +287,8 @@ static NTSTATUS create_sys_acl_blob(const struct
security_descriptor *psd,
>  			(ndr_push_flags_fn_t)ndr_push_xattr_NTACL);
>  
>  	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
> -		DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed:
%s\n",
> -			ndr_errstr(ndr_err)));
> +		DBG_INFO("ndr_push_xattr_NTACL failed: %s\n",
> +			 ndr_errstr(ndr_err));
>  		return ndr_map_error2ntstatus(ndr_err);
>  	}
>  
> @@ -345,10 +345,7 @@ static NTSTATUS
add_directory_inheritable_components(vfs_handle_struct *handle,
>  
>  	mode = dir_mode | file_mode;
>  
> -	DEBUG(10, ("add_directory_inheritable_components: directory %s,
"
> -		"mode = 0%o\n",
> -		name,
> -		(unsigned int)mode ));
> +	DBG_DEBUG("directory %s, mode = 0%o\n", name, (unsigned
int)mode);
>  
>  	if (num_aces) {
>  		memcpy(new_ace_list, psd->dacl->aces,
> @@ -880,7 +877,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  		smb_fname = smb_fname_in;
>  	}
>  
> -	DEBUG(10, ("get_nt_acl_internal: name=%s\n",
smb_fname->base_name));
> +	DBG_DEBUG("name=%s\n", smb_fname->base_name);
>  
>  	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
>  	if (NT_STATUS_IS_OK(status)) {
> @@ -1004,8 +1001,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct
*handle,
>  	}
>  
>  	if (DEBUGLEVEL >= 10) {
> -		DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
> -			smb_fname->base_name ));
> +		DBG_DEBUG("returning acl for %s is:\n",
> +			  smb_fname->base_name);
>  		NDR_PRINT_DEBUG(security_descriptor, psd);
>  	}
>  
> @@ -1072,9 +1069,8 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct
*handle, files_struct *fsp,
>  		return NT_STATUS_ACCESS_DENIED;
>  	}
>  
> -	DEBUG(10, ("fset_nt_acl_common: overriding chown on file %s "
> -		   "for sid %s\n",
> -		   fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid)));
> +	DBG_DEBUG("overriding chown on file %s for sid %s\n",
> +		   fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid));
>  
>  	/* Ok, we failed to chown and we have
>  	   SEC_STD_WRITE_OWNER access - override. */
> @@ -1097,27 +1093,25 @@ static NTSTATUS store_v3_blob(vfs_handle_struct
*handle, files_struct *fsp,
>  	DATA_BLOB blob;
>  
>  	if (DEBUGLEVEL >= 10) {
> -		DEBUG(10, ("fset_nt_acl_xattr: storing xattr sd for file
%s\n",
> -			   fsp_str_dbg(fsp)));
> +		DBG_DEBUG("storing xattr sd for file %s\n",
> +			  fsp_str_dbg(fsp));
>  		NDR_PRINT_DEBUG(
>  		    security_descriptor,
>  		    discard_const_p(struct security_descriptor, psd));
>  
>  		if (pdesc_next != NULL) {
> -			DEBUG(10, ("fset_nt_acl_xattr: storing has in xattr sd "
> -				   "based on \n"));
> +			DBG_DEBUG("storing xattr sd based on \n");
>  			NDR_PRINT_DEBUG(
>  			    security_descriptor,
>  			    discard_const_p(struct security_descriptor,
>  					    pdesc_next));
>  		} else {
> -			DEBUG(10,
> -			      ("fset_nt_acl_xattr: ignoring underlying sd\n"));
> +			DBG_DEBUG("ignoring underlying sd\n");
>  		}
>  	}
>  	status = create_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256,
hash);
>  	if (!NT_STATUS_IS_OK(status)) {
> -		DEBUG(10, ("fset_nt_acl_xattr: create_acl_blob failed\n"));
> +		DBG_DEBUG("create_acl_blob failed\n");
>  		return status;
>  	}
>  
> @@ -1146,8 +1140,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct
*handle, files_struct *fsp,
>  	    SNUM(handle->conn), ACL_MODULE_NAME, "ignore system
acls", false);
>  
>  	if (DEBUGLEVEL >= 10) {
> -		DEBUG(10,("fset_nt_acl_xattr: incoming sd for file %s\n",
> -			  fsp_str_dbg(fsp)));
> +		DBG_DEBUG("incoming sd for file %s\n", fsp_str_dbg(fsp));
>  		NDR_PRINT_DEBUG(security_descriptor,
>  			discard_const_p(struct security_descriptor, orig_psd));
>  	}
> @@ -1265,12 +1258,12 @@ static NTSTATUS
fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp,
>  	}
>  
>  	if (DEBUGLEVEL >= 10) {
> -		DEBUG(10,("fset_nt_acl_xattr: storing xattr sd for file %s based on
system ACL\n",
> -			  fsp_str_dbg(fsp)));
> +		DBG_DEBUG("storing xattr sd for file %s based on system
ACL\n",
> +			  fsp_str_dbg(fsp));
>  		NDR_PRINT_DEBUG(security_descriptor,
>  				discard_const_p(struct security_descriptor, psd));
>  
> -		DEBUG(10,("fset_nt_acl_xattr: storing hash in xattr sd based on
system ACL and:\n"));
> +		DBG_DEBUG("storing hash in xattr sd based on system ACL
and:\n");
>  		NDR_PRINT_DEBUG(security_descriptor,
>  				discard_const_p(struct security_descriptor, pdesc_next));
>  	}
> @@ -1282,7 +1275,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct
*handle, files_struct *fsp,
>  	status = create_sys_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256,
hash,
>  				     sys_acl_description, sys_acl_hash);
>  	if (!NT_STATUS_IS_OK(status)) {
> -		DEBUG(10, ("fset_nt_acl_xattr: create_sys_acl_blob
failed\n"));
> +		DBG_DEBUG("create_sys_acl_blob failed\n");
>  		TALLOC_FREE(frame);
>  		return status;
>  	}
> @@ -1319,9 +1312,8 @@ static int acl_common_remove_object(vfs_handle_struct
*handle,
>  		goto out;
>  	}
>  
> -	DEBUG(10,("acl_common_remove_object: removing %s %s/%s\n",
> -		is_directory ? "directory" : "file",
> -		parent_dir, final_component ));
> +	DBG_DEBUG("removing %s %s/%s\n", is_directory ?
"directory" : "file",
> +		  parent_dir, final_component);
>  
>   	/* cd into the parent dir to pin it. */
>  	ret = vfs_ChDir(conn, parent_dir);
> @@ -1354,10 +1346,9 @@ static int
acl_common_remove_object(vfs_handle_struct *handle,
>  	}
>  
>  	if (!fsp) {
> -		DEBUG(10,("acl_common_remove_object: %s %s/%s "
> -			"not an open file\n",
> -			is_directory ? "directory" : "file",
> -			parent_dir, final_component ));
> +		DBG_DEBUG("%s %s/%s not an open file\n",
> +			  is_directory ? "directory" : "file",
> +			  parent_dir, final_component);
>  		saved_errno = EACCES;
>  		goto out;
>  	}
> @@ -1405,9 +1396,9 @@ static int rmdir_acl_common(struct vfs_handle_struct
*handle,
>  						true);
>  	}
>  
> -	DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n",
> -		smb_fname->base_name,
> -		strerror(errno) ));
> +	DBG_DEBUG("unlink of %s failed %s\n",
> +		  smb_fname->base_name,
> +		  strerror(errno));
>  	return -1;
>  }
>  
> @@ -1434,9 +1425,9 @@ static int unlink_acl_common(struct vfs_handle_struct
*handle,
>  					false);
>  	}
>  
> -	DEBUG(10,("unlink_acl_common: unlink of %s failed %s\n",
> -		smb_fname->base_name,
> -		strerror(errno) ));
> +	DBG_DEBUG("unlink of %s failed %s\n",
> +		  smb_fname->base_name,
> +		  strerror(errno));
>  	return -1;
>  }
>  
> -- 
> 2.7.4
> 
> -- 
> To unsubscribe from this list go to the following URL and read the
> instructions:  https://lists.samba.org/mailman/options/samba