[Asterisk-code-review] res pjsip outbound authenticator digest.c: Fix memory pool l... (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Thu Nov 17 04:56:35 CST 2016
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4444 )
Change subject: res_pjsip_outbound_authenticator_digest.c: Fix memory pool leak.
......................................................................
res_pjsip_outbound_authenticator_digest.c: Fix memory pool leak.
Responding to authentication challenges leaks PJSIP memory pools.
The leak was introduced with a pjproject 2.5.5 API change.
https://trac.pjsip.org/repos/ticket/1929 changed the API usage of
pjsip_auth_clt_init() to require the new API pjsip_auth_clt_deinit() to
clean up cached authentication allocations that get allocated with
pjsip_auth_clt_reinit_req().
ASTERISK-26516 #close
Change-Id: I4473141b8c3961d0dc91c382beb3876b3efb45c8
---
M configure
M configure.ac
M include/asterisk/autoconfig.h.in
M res/res_pjsip_outbound_authenticator_digest.c
M third-party/pjproject/configure.m4
5 files changed, 140 insertions(+), 2 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
George Joseph: Looks good to me, but someone else must approve
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, approved
diff --git a/configure b/configure
index 5d21730..e90b352 100755
--- a/configure
+++ b/configure
@@ -943,6 +943,10 @@
POPT_DIR
POPT_INCLUDE
POPT_LIB
+PBX_PJSIP_AUTH_CLT_DEINIT
+PJSIP_AUTH_CLT_DEINIT_DIR
+PJSIP_AUTH_CLT_DEINIT_INCLUDE
+PJSIP_AUTH_CLT_DEINIT_LIB
PBX_PJSIP_INV_SESSION_REF
PJSIP_INV_SESSION_REF_DIR
PJSIP_INV_SESSION_REF_INCLUDE
@@ -9292,6 +9296,9 @@
$as_echo "#define HAVE_PJSIP_INV_SESSION_REF 1" >>confdefs.h
+$as_echo "#define HAVE_PJSIP_AUTH_CLT_DEINIT 1" >>confdefs.h
+
+
@@ -11456,6 +11463,18 @@
PJSIP_INV_SESSION_REF_DIR=${PJPROJECT_DIR}
PBX_PJSIP_INV_SESSION_REF=0
+
+
+
+
+
+
+
+PJSIP_AUTH_CLT_DEINIT_DESCRIP="pjsip_auth_clt_deinit support"
+PJSIP_AUTH_CLT_DEINIT_OPTION=pjsip
+PJSIP_AUTH_CLT_DEINIT_DIR=${PJPROJECT_DIR}
+
+PBX_PJSIP_AUTH_CLT_DEINIT=0
@@ -26423,6 +26442,110 @@
fi
+
+if test "x${PBX_PJSIP_AUTH_CLT_DEINIT}" != "x1" -a "${USE_PJSIP_AUTH_CLT_DEINIT}" != "no"; then
+ pbxlibdir=""
+ # if --with-PJSIP_AUTH_CLT_DEINIT=DIR has been specified, use it.
+ if test "x${PJSIP_AUTH_CLT_DEINIT_DIR}" != "x"; then
+ if test -d ${PJSIP_AUTH_CLT_DEINIT_DIR}/lib; then
+ pbxlibdir="-L${PJSIP_AUTH_CLT_DEINIT_DIR}/lib"
+ else
+ pbxlibdir="-L${PJSIP_AUTH_CLT_DEINIT_DIR}"
+ fi
+ fi
+ pbxfuncname="pjsip_auth_clt_deinit"
+ if test "x${pbxfuncname}" = "x" ; then # empty lib, assume only headers
+ AST_PJSIP_AUTH_CLT_DEINIT_FOUND=yes
+ else
+ ast_ext_lib_check_save_CFLAGS="${CFLAGS}"
+ CFLAGS="${CFLAGS} $PJPROJECT_CFLAGS"
+ as_ac_Lib=`$as_echo "ac_cv_lib_pjsip_${pbxfuncname}" | $as_tr_sh`
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for ${pbxfuncname} in -lpjsip" >&5
+$as_echo_n "checking for ${pbxfuncname} in -lpjsip... " >&6; }
+if eval \${$as_ac_Lib+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_check_lib_save_LIBS=$LIBS
+LIBS="-lpjsip ${pbxlibdir} $PJPROJECT_LIB $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char ${pbxfuncname} ();
+int
+main ()
+{
+return ${pbxfuncname} ();
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ eval "$as_ac_Lib=yes"
+else
+ eval "$as_ac_Lib=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+eval ac_res=\$$as_ac_Lib
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+if eval test \"x\$"$as_ac_Lib"\" = x"yes"; then :
+ AST_PJSIP_AUTH_CLT_DEINIT_FOUND=yes
+else
+ AST_PJSIP_AUTH_CLT_DEINIT_FOUND=no
+fi
+
+ CFLAGS="${ast_ext_lib_check_save_CFLAGS}"
+ fi
+
+ # now check for the header.
+ if test "${AST_PJSIP_AUTH_CLT_DEINIT_FOUND}" = "yes"; then
+ PJSIP_AUTH_CLT_DEINIT_LIB="${pbxlibdir} -lpjsip $PJPROJECT_LIB"
+ # if --with-PJSIP_AUTH_CLT_DEINIT=DIR has been specified, use it.
+ if test "x${PJSIP_AUTH_CLT_DEINIT_DIR}" != "x"; then
+ PJSIP_AUTH_CLT_DEINIT_INCLUDE="-I${PJSIP_AUTH_CLT_DEINIT_DIR}/include"
+ fi
+ PJSIP_AUTH_CLT_DEINIT_INCLUDE="${PJSIP_AUTH_CLT_DEINIT_INCLUDE} $PJPROJECT_CFLAGS"
+ if test "xpjsip.h" = "x" ; then # no header, assume found
+ PJSIP_AUTH_CLT_DEINIT_HEADER_FOUND="1"
+ else # check for the header
+ ast_ext_lib_check_saved_CPPFLAGS="${CPPFLAGS}"
+ CPPFLAGS="${CPPFLAGS} ${PJSIP_AUTH_CLT_DEINIT_INCLUDE}"
+ ac_fn_c_check_header_mongrel "$LINENO" "pjsip.h" "ac_cv_header_pjsip_h" "$ac_includes_default"
+if test "x$ac_cv_header_pjsip_h" = xyes; then :
+ PJSIP_AUTH_CLT_DEINIT_HEADER_FOUND=1
+else
+ PJSIP_AUTH_CLT_DEINIT_HEADER_FOUND=0
+fi
+
+
+ CPPFLAGS="${ast_ext_lib_check_saved_CPPFLAGS}"
+ fi
+ if test "x${PJSIP_AUTH_CLT_DEINIT_HEADER_FOUND}" = "x0" ; then
+ PJSIP_AUTH_CLT_DEINIT_LIB=""
+ PJSIP_AUTH_CLT_DEINIT_INCLUDE=""
+ else
+ if test "x${pbxfuncname}" = "x" ; then # only checking headers -> no library
+ PJSIP_AUTH_CLT_DEINIT_LIB=""
+ fi
+ PBX_PJSIP_AUTH_CLT_DEINIT=1
+ cat >>confdefs.h <<_ACEOF
+#define HAVE_PJSIP_AUTH_CLT_DEINIT 1
+_ACEOF
+
+ fi
+ fi
+fi
+
+
fi
fi
diff --git a/configure.ac b/configure.ac
index ea55df4..db21fb3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -510,6 +510,7 @@
AST_EXT_LIB_SETUP_OPTIONAL([PJSIP_TLS_TRANSPORT_PROTO], [PJSIP TLS Transport proto field support], [PJPROJECT], [pjsip])
AST_EXT_LIB_SETUP_OPTIONAL([PJSIP_EVSUB_GRP_LOCK], [PJSIP EVSUB Group Lock support], [PJPROJECT], [pjsip])
AST_EXT_LIB_SETUP_OPTIONAL([PJSIP_INV_SESSION_REF], [PJSIP INVITE Session Reference Count support], [PJPROJECT], [pjsip])
+AST_EXT_LIB_SETUP_OPTIONAL([PJSIP_AUTH_CLT_DEINIT], [pjsip_auth_clt_deinit support], [PJPROJECT], [pjsip])
fi
AST_EXT_LIB_SETUP([POPT], [popt], [popt])
@@ -2233,6 +2234,7 @@
AST_EXT_LIB_CHECK([PJSIP_EVSUB_GRP_LOCK], [pjsip], [pjsip_evsub_add_ref], [pjsip.h], [$PJPROJECT_LIB], [$PJPROJECT_CFLAGS])
AST_EXT_LIB_CHECK([PJSIP_INV_SESSION_REF], [pjsip], [pjsip_inv_add_ref], [pjsip.h], [$PJPROJECT_LIB], [$PJPROJECT_CFLAGS])
+ AST_EXT_LIB_CHECK([PJSIP_AUTH_CLT_DEINIT], [pjsip], [pjsip_auth_clt_deinit], [pjsip.h], [$PJPROJECT_LIB], [$PJPROJECT_CFLAGS])
fi
fi
diff --git a/include/asterisk/autoconfig.h.in b/include/asterisk/autoconfig.h.in
index 706798c..bac2ecf 100644
--- a/include/asterisk/autoconfig.h.in
+++ b/include/asterisk/autoconfig.h.in
@@ -592,6 +592,9 @@
/* Define if your system has PJPROJECT_BUNDLED */
#undef HAVE_PJPROJECT_BUNDLED
+/* Define to 1 if PJPROJECT has the pjsip_auth_clt_deinit support feature. */
+#undef HAVE_PJSIP_AUTH_CLT_DEINIT
+
/* Define to 1 if PJPROJECT has the PJSIP Dialog Create UAS with Incremented
Lock feature. */
#undef HAVE_PJSIP_DLG_CREATE_UAS_AND_INC_LOCK
diff --git a/res/res_pjsip_outbound_authenticator_digest.c b/res/res_pjsip_outbound_authenticator_digest.c
index 86a15c7..ce77c3b 100644
--- a/res/res_pjsip_outbound_authenticator_digest.c
+++ b/res/res_pjsip_outbound_authenticator_digest.c
@@ -106,6 +106,7 @@
{
pjsip_auth_clt_sess auth_sess;
pjsip_cseq_hdr *cseq;
+ pj_status_t status;
if (pjsip_auth_clt_init(&auth_sess, ast_sip_get_pjsip_endpoint(),
old_request->pool, 0) != PJ_SUCCESS) {
@@ -115,11 +116,19 @@
if (set_outbound_authentication_credentials(&auth_sess, auths, challenge)) {
ast_log(LOG_WARNING, "Failed to set authentication credentials\n");
+#if defined(HAVE_PJSIP_AUTH_CLT_DEINIT)
+ /* In case it is not a noop here in the future. */
+ pjsip_auth_clt_deinit(&auth_sess);
+#endif
return -1;
}
- switch (pjsip_auth_clt_reinit_req(&auth_sess, challenge,
- old_request, new_request)) {
+ status = pjsip_auth_clt_reinit_req(&auth_sess, challenge, old_request, new_request);
+#if defined(HAVE_PJSIP_AUTH_CLT_DEINIT)
+ /* Release any cached auths */
+ pjsip_auth_clt_deinit(&auth_sess);
+#endif
+ switch (status) {
case PJ_SUCCESS:
/* PJSIP creates a new transaction for new_request (meaning it creates a new
* branch). However, it recycles the Call-ID, from-tag, and CSeq from the
diff --git a/third-party/pjproject/configure.m4 b/third-party/pjproject/configure.m4
index 8704682..7c60c2a 100644
--- a/third-party/pjproject/configure.m4
+++ b/third-party/pjproject/configure.m4
@@ -55,6 +55,7 @@
AC_DEFINE([HAVE_PJSIP_TLS_TRANSPORT_PROTO], 1, [Define if your system has PJSIP_TLS_TRANSPORT_PROTO])
AC_DEFINE([HAVE_PJSIP_EVSUB_GRP_LOCK], 1, [Define if your system has PJSIP_EVSUB_GRP_LOCK])
AC_DEFINE([HAVE_PJSIP_INV_SESSION_REF], 1, [Define if your system has PJSIP_INV_SESSION_REF])
+ AC_DEFINE([HAVE_PJSIP_AUTH_CLT_DEINIT], 1, [Define if your system has pjsip_auth_clt_deinit declared.])
AC_SUBST([PJPROJECT_BUNDLED])
AC_SUBST([PJPROJECT_DIR])
--
To view, visit https://gerrit.asterisk.org/4444
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4473141b8c3961d0dc91c382beb3876b3efb45c8
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list