[PATCH] Fix several memory and resource leaks

[Andreas Schneider]

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
-------------- next part --------------
>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