[Patches] Preparation for tevent impersonation (part1)

[Jeremy Allison]

On Thu, Jun 14, 2018 at 10:12:45AM -0700, Jeremy Allison wrote:
> On Thu, Jun 14, 2018 at 10:03:42AM -0700, Jeremy Allison via samba-technical wrote:
> > On Thu, Jun 14, 2018 at 06:48:39PM +0200, Stefan Metzmacher wrote:
> > > Hi Jeremy,
> > > 
> > > > OK, finishing up the review with this patch inserted
> > > > before #6 in the previous set. Should get this finished
> > > > (and pushed) tomorrow.
> > > 
> > > Here's an updated patchset.
> > > 
> > > The author of the first two commits is fixed and the commit message
> > > of the last one is extended (as requested by Andreas).
> > 
> > FYI: Descriptions for patches 22/41 and 23/41 are incorrect,
> > missing signed-off-by in patch 37/41.
> > 
> > Other than that RB+ - looks good to me.
> > 
> > However I'm getting an interesting autobuild failure
> > when I push:
> > 
> > I'm looking into it !
> 
> Found it !
> 
> source3/smbd/pysmbd.c:static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
> 
> return an ACL created on a talloc stackframe
> that is freed. The additional patch is needed
> I think. Can you add on top and review ?

Mea culpa, the crash was caused by my additional

TALLOC_FREE(frame);

change in make_simple_acl(). Having said that
the stackframe handling in make_simple_acl()
was utterly broken before that anyway (it
created a new stackframe that was left
hanging around on exit from the function).

Jeremy.


> From cd43ba5f76824807f5e93a8ad2543bea74fb464a Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Thu, 14 Jun 2018 10:12:03 -0700
> Subject: [PATCH] pysmbd: Don't return an ACL on a freed talloc_stackframe.
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/smbd/pysmbd.c | 34 ++++++++++------------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
> index 4e4299ca5d7..95325098bb6 100644
> --- a/source3/smbd/pysmbd.c
> +++ b/source3/smbd/pysmbd.c
> @@ -260,106 +260,88 @@ static int set_acl_entry_perms(SMB_ACL_ENTRY_T entry, mode_t perm_mask)
>  	return 0;
>  }
>  
> -static SMB_ACL_T make_simple_acl(gid_t gid, mode_t chmod_mode)
> +static SMB_ACL_T make_simple_acl(TALLOC_CTX *mem_ctx,
> +		gid_t gid,
> +		mode_t chmod_mode)
>  {
> -	TALLOC_CTX *frame = talloc_stackframe();
> -
>  	mode_t mode = SMB_ACL_READ|SMB_ACL_WRITE|SMB_ACL_EXECUTE;
>  
>  	mode_t mode_user = (chmod_mode & 0700) >> 6;
>  	mode_t mode_group = (chmod_mode & 070) >> 3;
>  	mode_t mode_other = chmod_mode &  07;
>  	SMB_ACL_ENTRY_T entry;
> -	SMB_ACL_T acl = sys_acl_init(frame);
> +	SMB_ACL_T acl = sys_acl_init(mem_ctx);
>  
>  	if (!acl) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_create_entry(&acl, &entry) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_set_tag_type(entry, SMB_ACL_USER_OBJ) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (set_acl_entry_perms(entry, mode_user) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_create_entry(&acl, &entry) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_set_tag_type(entry, SMB_ACL_GROUP_OBJ) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (set_acl_entry_perms(entry, mode_group) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_create_entry(&acl, &entry) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_set_tag_type(entry, SMB_ACL_OTHER) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (set_acl_entry_perms(entry, mode_other) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (gid != -1) {
>  		if (sys_acl_create_entry(&acl, &entry) != 0) {
> -			TALLOC_FREE(frame);
>  			return NULL;
>  		}
>  
>  		if (sys_acl_set_tag_type(entry, SMB_ACL_GROUP) != 0) {
> -			TALLOC_FREE(frame);
>  			return NULL;
>  		}
>  
>  		if (sys_acl_set_qualifier(entry, &gid) != 0) {
> -			TALLOC_FREE(frame);
>  			return NULL;
>  		}
>  
>  		if (set_acl_entry_perms(entry, mode_group) != 0) {
> -			TALLOC_FREE(frame);
>  			return NULL;
>  		}
>  	}
>  
>  	if (sys_acl_create_entry(&acl, &entry) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (sys_acl_set_tag_type(entry, SMB_ACL_MASK) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
>  	if (set_acl_entry_perms(entry, mode) != 0) {
> -		TALLOC_FREE(frame);
>  		return NULL;
>  	}
>  
> -	TALLOC_FREE(frame);
>  	return acl;
>  }
>  
> @@ -381,10 +363,14 @@ static PyObject *py_smbd_set_simple_acl(PyObject *self, PyObject *args, PyObject
>  					 &fname, &mode, &gid, &service))
>  		return NULL;
>  
> -	acl = make_simple_acl(gid, mode);
> -
>  	frame = talloc_stackframe();
>  
> +	acl = make_simple_acl(frame, gid, mode);
> +	if (acl == NULL) {
> +		TALLOC_FREE(frame);
> +		return NULL;
> +	}
> +
>  	conn = get_conn_tos(service);
>  	if (!conn) {
>  		TALLOC_FREE(frame);
> -- 
> 2.18.0.rc1.244.gcf134e6275-goog
>