[PATCH] Fix several memory and resource leaks

[Jeremy Allison]

On Mon, Aug 13, 2018 at 02:20:15PM +0200, Andreas Schneider wrote:
> On Sunday, 12 August 2018 15:40:21 CEST Volker Lendecke wrote:
> > On Fri, Aug 10, 2018 at 04:36:43PM +0200, Andreas Schneider wrote:
> > > I would keep the patch as it is, but would add an additional patch for
> > > master which uses a talloc_strackframe(). This would mean we rewrite the
> > > code and pass a talloc memory context to parse_nss_parm(). Then we just
> > > need one TALLOC_FREE(frame) before we leave.
> > 
> > Just push it as you like it with my RB+.
> > 
> > Sorry for the noise,
> > 
> > Volker
> 
> 
> -- 
> Andreas Schneider                      asn at samba.org
> Samba Team                             www.samba.org
> GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D

LGTM Andreas, thanks for the fixup !

Jeremy.

> From 2f721dd77b919d219c049165b66ad919f6b106f9 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Thu, 9 Aug 2018 16:38:49 +0200
> Subject: [PATCH] s3:winbind: Fix memory leak in nss_init()
> 
> Found by covscan.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13567
> 
> Pair-Programmed-With: Justin Stephenson <jstephen at redhat.com>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Signed-off-by: Justin Stephenson <jstephen at redhat.com>
> ---
>  source3/winbindd/nss_info.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/source3/winbindd/nss_info.c b/source3/winbindd/nss_info.c
> index 473b1a3b66e..1a8325ce7dc 100644
> --- a/source3/winbindd/nss_info.c
> +++ b/source3/winbindd/nss_info.c
> @@ -84,7 +84,10 @@ static struct nss_function_entry *nss_get_backend(const char *name )
>  /********************************************************************
>   *******************************************************************/
>  
> -static bool parse_nss_parm( const char *config, char **backend, char **domain )
> +static bool parse_nss_parm(TALLOC_CTX *mem_ctx,
> +			   const char *config,
> +			   char **backend,
> +			   char **domain)
>  {
>  	char *p;
>  
> @@ -98,17 +101,17 @@ static bool parse_nss_parm( const char *config, char **backend, char **domain )
>  	/* if no : then the string must be the backend name only */
>  
>  	if ( !p ) {
> -		*backend = SMB_STRDUP( config );
> +		*backend = talloc_strdup(mem_ctx, config);
>  		return (*backend != NULL);
>  	}
>  
>  	/* split the string and return the two parts */
>  
>  	if ( strlen(p+1) > 0 ) {
> -		*domain = SMB_STRDUP( p+1 );
> +		*domain = talloc_strdup(mem_ctx, p + 1);
>  	}
>  
> -	*backend = SMB_STRNDUP(config, PTR_DIFF(p, config));
> +	*backend = talloc_strndup(mem_ctx, config, PTR_DIFF(p, config));
>  	return (*backend != NULL);
>  }
>  
> @@ -158,8 +161,9 @@ static NTSTATUS nss_init(const char **nss_list)
>  	NTSTATUS status;
>  	static bool nss_initialized = false;
>  	int i;
> -	char *backend, *domain;
> +	char *backend = NULL, *domain = NULL;
>  	struct nss_function_entry *nss_backend;
> +	TALLOC_CTX *frame;
>  
>  	/* check for previous successful initializations */
>  
> @@ -167,6 +171,8 @@ static NTSTATUS nss_init(const char **nss_list)
>  		return NT_STATUS_OK;
>  	}
>  
> +	frame = talloc_stackframe();
> +
>  	/* The "template" backend should always be registered as it
>  	   is a static module */
>  
> @@ -179,8 +185,10 @@ static NTSTATUS nss_init(const char **nss_list)
>  	   as necessary) */
>  
>  	for ( i=0; nss_list && nss_list[i]; i++ ) {
> +		bool ok;
>  
> -		if ( !parse_nss_parm(nss_list[i], &backend, &domain) ) {
> +		ok = parse_nss_parm(frame, nss_list[i], &backend, &domain);
> +		if (!ok) {
>  			DEBUG(0,("nss_init: failed to parse \"%s\"!\n",
>  				 nss_list[i]));
>  			continue;
> @@ -238,10 +246,11 @@ static NTSTATUS nss_init(const char **nss_list)
>  
>  		/* cleanup */
>  
> -		SAFE_FREE( backend );
> -		SAFE_FREE( domain );
> +		TALLOC_FREE(domain);
> +		TALLOC_FREE(backend);
>  	}
>  
> +
>  	if ( !nss_domain_list ) {
>  		DEBUG(3,("nss_init: no nss backends configured.  "
>  			 "Defaulting to \"template\".\n"));
> @@ -252,6 +261,7 @@ static NTSTATUS nss_init(const char **nss_list)
>  
>  	nss_initialized = true;
>  
> +	TALLOC_FREE(frame);
>  	return NT_STATUS_OK;
>  }
>  
> -- 
> 2.18.0
>