From 732447a070f9b30283bbca2b98817f28963fca57 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 1 Apr 2014 17:01:26 +1300 Subject: [PATCH 1/3] 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 Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison (cherry picked from commit 8f3a516acb8c95cd6d88bf80abd495ac0cafaae3) --- 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.3 From 7e0ee950db5c51cd35f92f85bd465ee1c8f54a0c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 1 Apr 2014 17:03:34 +1300 Subject: [PATCH 2/3] libsmbclient: Wrap more function calls in talloc_stackframe() to protect against talloc_tos() calls BUG: https://bugzilla.samba.org/show_bug.cgi?id=8449 Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Wed Apr 2 02:36:08 CEST 2014 on sn-devel-104 (cherry picked from commit 014342746f5af1aaaf1c2f8b44098c3a944e4f0a) --- source3/libsmb/libsmb_context.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/source3/libsmb/libsmb_context.c b/source3/libsmb/libsmb_context.c index c2b88f5..ffa4d2d 100644 --- a/source3/libsmb/libsmb_context.c +++ b/source3/libsmb/libsmb_context.c @@ -560,6 +560,7 @@ SMBCCTX * smbc_init_context(SMBCCTX *context) { int pid; + TALLOC_CTX *frame; if (!context) { errno = EBADF; @@ -571,11 +572,14 @@ smbc_init_context(SMBCCTX *context) return NULL; } + frame = talloc_stackframe(); + if ((!smbc_getFunctionAuthData(context) && !smbc_getFunctionAuthDataWithContext(context)) || smbc_getDebug(context) < 0 || smbc_getDebug(context) > 100) { + TALLOC_FREE(frame); errno = EINVAL; return NULL; @@ -594,6 +598,7 @@ smbc_init_context(SMBCCTX *context) } if (!user) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -602,6 +607,7 @@ smbc_init_context(SMBCCTX *context) SAFE_FREE(user); if (!smbc_getUser(context)) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -624,6 +630,7 @@ smbc_init_context(SMBCCTX *context) pid = getpid(); netbios_name = (char *)SMB_MALLOC(17); if (!netbios_name) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -632,6 +639,7 @@ smbc_init_context(SMBCCTX *context) } if (!netbios_name) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -640,6 +648,7 @@ smbc_init_context(SMBCCTX *context) SAFE_FREE(netbios_name); if (!smbc_getNetbiosName(context)) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -659,6 +668,7 @@ smbc_init_context(SMBCCTX *context) } if (!workgroup) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -667,6 +677,7 @@ smbc_init_context(SMBCCTX *context) SAFE_FREE(workgroup); if (!smbc_getWorkgroup(context)) { + TALLOC_FREE(frame); errno = ENOMEM; return NULL; } @@ -692,6 +703,7 @@ smbc_init_context(SMBCCTX *context) smb_panic("error unlocking 'initialized_ctx_count'"); } + TALLOC_FREE(frame); return context; } @@ -727,12 +739,15 @@ void smbc_set_credentials_with_fallback(SMBCCTX *context, smbc_bool use_kerberos = false; const char *signing_state = "off"; struct user_auth_info *auth_info = NULL; + TALLOC_CTX *frame; if (! context) { return; } + frame = talloc_stackframe(); + if (! workgroup || ! *workgroup) { workgroup = smbc_getWorkgroup(context); } @@ -749,6 +764,7 @@ void smbc_set_credentials_with_fallback(SMBCCTX *context, if (! auth_info) { DEBUG(0, ("smbc_set_credentials_with_fallback: allocation fail\n")); + TALLOC_FREE(frame); return; } @@ -777,4 +793,5 @@ void smbc_set_credentials_with_fallback(SMBCCTX *context, TALLOC_FREE(context->internal->auth_info); context->internal->auth_info = auth_info; + TALLOC_FREE(frame); } -- 1.9.3 From 0b6fedad87962d6962061e193aa2ae13153c1169 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 31 Mar 2014 10:19:58 +1300 Subject: [PATCH 3/3] libsmb: Provide a talloc_stackframe() to external users of libsmb_setget.c Signed-off-by: Andrew Bartlett Reviewed-by: Jeremy Allison (cherry picked from commit bc5bd4010e8fedf19047ed6f7a793cd373f9f14f) --- source3/libsmb/libsmb_setget.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/source3/libsmb/libsmb_setget.c b/source3/libsmb/libsmb_setget.c index b8adcca..3255b52 100644 --- a/source3/libsmb/libsmb_setget.c +++ b/source3/libsmb/libsmb_setget.c @@ -91,9 +91,11 @@ void smbc_setDebug(SMBCCTX *c, int debug) { char buf[32]; + TALLOC_CTX *frame = talloc_stackframe(); snprintf(buf, sizeof(buf), "%d", debug); c->debug = debug; lp_set_cmdline("log level", buf); + TALLOC_FREE(frame); } /** @@ -139,10 +141,15 @@ smbc_setPort(SMBCCTX *c, uint16_t port) smbc_bool smbc_getOptionDebugToStderr(SMBCCTX *c) { + smbc_bool ret; + TALLOC_CTX *frame = talloc_stackframe(); + /* Because this is a global concept, it is better to check * what is really set, rather than what we wanted set * (particularly as you cannot go back to stdout). */ - return debug_get_output_is_stderr(); + ret = debug_get_output_is_stderr(); + TALLOC_FREE(frame); + return ret; } /** Set whether to log to standard error instead of standard output. @@ -154,6 +161,7 @@ smbc_getOptionDebugToStderr(SMBCCTX *c) void smbc_setOptionDebugToStderr(SMBCCTX *c, smbc_bool b) { + TALLOC_CTX *frame = talloc_stackframe(); if (b) { /* * We do not have a unique per-thread debug state? For @@ -164,6 +172,7 @@ smbc_setOptionDebugToStderr(SMBCCTX *c, smbc_bool b) */ setup_logging("libsmbclient", DEBUG_STDERR); } + TALLOC_FREE(frame); } /** @@ -498,7 +507,11 @@ smbc_setOptionUseNTHash(SMBCCTX *c, smbc_bool b) smbc_get_auth_data_fn smbc_getFunctionAuthData(SMBCCTX *c) { - return c->callbacks.auth_fn; + smbc_get_auth_data_fn ret; + TALLOC_CTX *frame = talloc_stackframe(); + ret = c->callbacks.auth_fn; + TALLOC_FREE(frame); + return ret; } /** Set the function for obtaining authentication data */ -- 1.9.3