On Fri, 2014-01-03 at 17:54 -0500, Mattthew Gabeler-Lee wrote:
> Package: libpam-smbpass
> Version: 2:4.0.13+dfsg-1
> Followup-For: Bug #728666
> 
> After searching around, I found this bug on the samba bugzilla that hints at 
> how to fix this problem, though ti seems like something that upstream ought 
> to do.
> 
> https://bugzilla.samba.org/show_bug.cgi?id=8449

If someone would like to try the attached patch and confirm I've not
broken anything (ie test all the auth/acct/password steps), that would
be most helpful. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba



>From 6e5a0f69f91a63143c60157bd084a5c89b03d261 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abart...@samba.org>
Date: Tue, 1 Apr 2014 17:01:26 +1300
Subject: [PATCH 7/8] pam_smbpass: Wrap calls in talloc_stackframe() to avoid
 warnings about leaking memory

Any code in source3 is permitted to use talloc_tos() at any point, so we must protect all the library interfaces
against memory leaks this way.

Andrew Bartlett

Change-Id: I71c805caebbc41a8b0a6ce09a5afa2c64b87361b
Signed-off-by: Andrew Bartlett <abart...@samba.org>
---
 source3/pam_smbpass/pam_smb_acct.c   |  9 +++++++++
 source3/pam_smbpass/pam_smb_auth.c   |  7 +++++++
 source3/pam_smbpass/pam_smb_passwd.c | 16 +++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/source3/pam_smbpass/pam_smb_acct.c b/source3/pam_smbpass/pam_smb_acct.c
index 60acd3c..bd4615f 100644
--- a/source3/pam_smbpass/pam_smb_acct.c
+++ b/source3/pam_smbpass/pam_smb_acct.c
@@ -55,6 +55,7 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 	const char *name;
 	struct samu *sampass = NULL;
 	void (*oldsig_handler)(int);
+	TALLOC_CTX *frame = talloc_stackframe();
 
 	/* Samba initialization. */
 	load_case_tables_library();
@@ -68,6 +69,7 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 		if (on( SMB_DEBUG, ctrl )) {
 			_log_err(pamh, LOG_DEBUG, "acct: could not identify user" );
 		}
+		TALLOC_FREE(frame);
 		return retval;
 	}
 	if (on( SMB_DEBUG, ctrl )) {
@@ -76,6 +78,7 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 
 	if (geteuid() != 0) {
 		_log_err(pamh, LOG_DEBUG, "Cannot access samba password database, not running as root.");
+		TALLOC_FREE(frame);
 		return PAM_AUTHINFO_UNAVAIL;
 	}
 
@@ -85,6 +88,7 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 	if (!initialize_password_db(True, NULL)) {
 	  _log_err(pamh, LOG_ALERT, "Cannot access samba password database" );
 		CatchSignal(SIGPIPE, oldsig_handler);
+		TALLOC_FREE(frame);
 		return PAM_AUTHINFO_UNAVAIL;
 	}
 
@@ -93,18 +97,21 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 	if (!(sampass = samu_new( NULL ))) {
 		CatchSignal(SIGPIPE, oldsig_handler);
 		/* malloc fail. */
+		TALLOC_FREE(frame);
 		return nt_status_to_pam(NT_STATUS_NO_MEMORY);
 	}
 
 	if (!pdb_getsampwnam(sampass, name )) {
 		_log_err(pamh, LOG_DEBUG, "acct: could not identify user");
 		CatchSignal(SIGPIPE, oldsig_handler);
+		TALLOC_FREE(frame);
         	return PAM_USER_UNKNOWN;
 	}
 
 	/* check for lookup failure */
 	if (!strlen(pdb_get_username(sampass)) ) {
 		CatchSignal(SIGPIPE, oldsig_handler);
+		TALLOC_FREE(frame);
 		return PAM_USER_UNKNOWN;
 	}
 
@@ -118,12 +125,14 @@ int pam_sm_acct_mgmt( pam_handle_t *pamh, int flags,
 			"please see your system administrator." );
 
 		CatchSignal(SIGPIPE, oldsig_handler);
+		TALLOC_FREE(frame);
 		return PAM_ACCT_EXPIRED;
 	}
 
 	/* TODO: support for expired passwords. */
 
 	CatchSignal(SIGPIPE, oldsig_handler);
+	TALLOC_FREE(frame);
 	return PAM_SUCCESS;
 }
 
diff --git a/source3/pam_smbpass/pam_smb_auth.c b/source3/pam_smbpass/pam_smb_auth.c
index 4270bcc..ac5ef3f 100644
--- a/source3/pam_smbpass/pam_smb_auth.c
+++ b/source3/pam_smbpass/pam_smb_auth.c
@@ -50,6 +50,7 @@ do {								\
 		pam_set_data( pamh, "smb_setcred_return"	\
 		              , (void *) ret_data, NULL );	\
 	}							\
+	TALLOC_FREE(frame);					\
 	return retval;						\
 } while (0)
 
@@ -75,6 +76,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, int flags,
 	const char *name;
 	void (*oldsig_handler)(int) = NULL;
 	bool found;
+	TALLOC_CTX *frame = talloc_stackframe();
 
 	/* Points to memory managed by the PAM library. Do not free. */
 	char *p = NULL;
@@ -195,6 +197,7 @@ static int _smb_add_user(pam_handle_t *pamh, unsigned int ctrl,
 	char *msg_str = NULL;
 	const char *pass = NULL;
 	int retval;
+	TALLOC_CTX *frame = talloc_stackframe();
 
 	/* Get the authtok; if we don't have one, silently fail. */
 	retval = _pam_get_item( pamh, PAM_AUTHTOK, &pass );
@@ -202,8 +205,10 @@ static int _smb_add_user(pam_handle_t *pamh, unsigned int ctrl,
 	if (retval != PAM_SUCCESS) {
 		_log_err(pamh, LOG_ALERT
 			, "pam_get_item returned error to pam_sm_authenticate" );
+		TALLOC_FREE(frame);
 		return PAM_AUTHTOK_RECOVER_ERR;
 	} else if (pass == NULL) {
+		TALLOC_FREE(frame);
 		return PAM_AUTHTOK_RECOVER_ERR;
 	}
 
@@ -220,6 +225,7 @@ static int _smb_add_user(pam_handle_t *pamh, unsigned int ctrl,
 
 		SAFE_FREE(err_str);
 		SAFE_FREE(msg_str);
+		TALLOC_FREE(frame);
 		return PAM_IGNORE;
 	} else {
 		/* mimick 'update encrypted' as long as the 'no pw req' flag is not set */
@@ -237,6 +243,7 @@ static int _smb_add_user(pam_handle_t *pamh, unsigned int ctrl,
 	SAFE_FREE(err_str);
 	SAFE_FREE(msg_str);
 	pass = NULL;
+	TALLOC_FREE(frame);
 	return PAM_IGNORE;
 }
 
diff --git a/source3/pam_smbpass/pam_smb_passwd.c b/source3/pam_smbpass/pam_smb_passwd.c
index ce0b118..dedfda0 100644
--- a/source3/pam_smbpass/pam_smb_passwd.c
+++ b/source3/pam_smbpass/pam_smb_passwd.c
@@ -103,6 +103,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
     const char *user;
     char *pass_old;
     char *pass_new;
+    TALLOC_CTX *frame = talloc_stackframe();
 
     /* Samba initialization. */
     load_case_tables_library();
@@ -119,6 +120,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
         if (on( SMB_DEBUG, ctrl )) {
             _log_err(pamh, LOG_DEBUG, "password: could not identify user");
         }
+	TALLOC_FREE(frame);
         return retval;
     }
     if (on( SMB_DEBUG, ctrl )) {
@@ -127,6 +129,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
 
     if (geteuid() != 0) {
 	_log_err(pamh, LOG_DEBUG, "Cannot access samba password database, not running as root.");
+	TALLOC_FREE(frame);
 	return PAM_AUTHINFO_UNAVAIL;
     }
 
@@ -137,19 +140,22 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
     if (!initialize_password_db(False, NULL)) {
       _log_err(pamh, LOG_ALERT, "Cannot access samba password database" );
         CatchSignal(SIGPIPE, oldsig_handler);
+	TALLOC_FREE(frame);
         return PAM_AUTHINFO_UNAVAIL;
     }
 
     /* obtain user record */
     if ( !(sampass = samu_new( NULL )) ) {
         CatchSignal(SIGPIPE, oldsig_handler);
+	TALLOC_FREE(frame);
         return nt_status_to_pam(NT_STATUS_NO_MEMORY);
     }
 
     if (!pdb_getsampwnam(sampass,user)) {
         _log_err(pamh, LOG_ALERT, "Failed to find entry for user %s.", user);
         CatchSignal(SIGPIPE, oldsig_handler);
-        return PAM_USER_UNKNOWN;
+	TALLOC_FREE(frame);
+	return PAM_USER_UNKNOWN;
     }
     if (on( SMB_DEBUG, ctrl )) {
         _log_err(pamh, LOG_DEBUG, "Located account for %s", user);
@@ -167,6 +173,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
 
             TALLOC_FREE(sampass);
             CatchSignal(SIGPIPE, oldsig_handler);
+	    TALLOC_FREE(frame);
             return PAM_SUCCESS;
         }
 
@@ -179,6 +186,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
 			_log_err(pamh, LOG_CRIT, "password: out of memory");
 			TALLOC_FREE(sampass);
 			CatchSignal(SIGPIPE, oldsig_handler);
+			TALLOC_FREE(frame);
 			return PAM_BUF_ERR;
 		}
 
@@ -192,6 +200,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
                          "password - (old) token not obtained");
                 TALLOC_FREE(sampass);
                 CatchSignal(SIGPIPE, oldsig_handler);
+		TALLOC_FREE(frame);
                 return retval;
             }
 
@@ -207,6 +216,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
         pass_old = NULL;
         TALLOC_FREE(sampass);
         CatchSignal(SIGPIPE, oldsig_handler);
+	TALLOC_FREE(frame);
         return retval;
 
     } else if (flags & PAM_UPDATE_AUTHTOK) {
@@ -237,6 +247,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
             _log_err(pamh, LOG_NOTICE, "password: user not authenticated");
             TALLOC_FREE(sampass);
             CatchSignal(SIGPIPE, oldsig_handler);
+	    TALLOC_FREE(frame);
             return retval;
         }
 
@@ -265,6 +276,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
             pass_old = NULL;                               /* tidy up */
             TALLOC_FREE(sampass);
             CatchSignal(SIGPIPE, oldsig_handler);
+	    TALLOC_FREE(frame);
             return retval;
         }
 
@@ -285,6 +297,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
             pass_new = pass_old = NULL;               /* tidy up */
             TALLOC_FREE(sampass);
             CatchSignal(SIGPIPE, oldsig_handler);
+	    TALLOC_FREE(frame);
             return retval;
         }
 
@@ -334,6 +347,7 @@ int pam_sm_chauthtok(pam_handle_t *pamh, int flags,
 
     TALLOC_FREE(sampass);
     CatchSignal(SIGPIPE, oldsig_handler);
+    TALLOC_FREE(frame);
     return retval;
 }
 
-- 
1.9.1

Reply via email to