From 00de10c0d1a07ba27c50eb224c4a200e5f89b88c Mon Sep 17 00:00:00 2001 From: George Joseph Date: Thu, 23 Jan 2025 14:02:25 -0700 Subject: [PATCH] res_pjsip: Fix startup/reload memory leak in config_auth. An issue in config_auth.c:ast_sip_auth_digest_algorithms_vector_init() was causing double allocations for the two supported_algorithms vectors to the tune of 915 bytes. The leak only happens on startup and when a reload is done and doesn't get bigger with the number of auth objects defined. * Pre-initialized the two vectors in config_auth:auth_alloc(). * Removed the allocations in ast_sip_auth_digest_algorithms_vector_init(). * Added a note to the doc for ast_sip_auth_digest_algorithms_vector_init() noting that the vector passed in should be initialized and empty. * Simplified the create_artificial_auth() function in pjsip_distributor. * Set the vector initialization count to 0 in config_global:global_apply(). --- include/asterisk/res_pjsip.h | 2 +- res/res_pjsip/config_auth.c | 12 +++------- res/res_pjsip/config_global.c | 4 ++-- res/res_pjsip/pjsip_distributor.c | 37 +++++++------------------------ 4 files changed, 14 insertions(+), 41 deletions(-) diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h index 0a0724db21..2fafd5790a 100644 --- a/include/asterisk/res_pjsip.h +++ b/include/asterisk/res_pjsip.h @@ -1333,7 +1333,7 @@ enum ast_sip_check_auth_result { * \brief Populate a vector of algorithm types from a string. * * \param id The object id to use in error messages - * \param algorithms The vector to populate + * \param algorithms The initialized but empty vector to populate * \param agent_type The type of agent to use in error messages ("UAC" or "UAS") * \param value The comma-separated string to parse for algorithms * diff --git a/res/res_pjsip/config_auth.c b/res/res_pjsip/config_auth.c index c55d3b2e8e..53f9963489 100644 --- a/res/res_pjsip/config_auth.c +++ b/res/res_pjsip/config_auth.c @@ -139,6 +139,9 @@ static void *auth_alloc(const char *name) return NULL; } + AST_VECTOR_INIT(&auth->supported_algorithms_uac, 0); + AST_VECTOR_INIT(&auth->supported_algorithms_uas, 0); + return auth; } @@ -196,13 +199,6 @@ int ast_sip_auth_digest_algorithms_vector_init(const char *id, ast_assert(algorithms != NULL); - if (AST_VECTOR_SIZE(algorithms)) { - AST_VECTOR_FREE(algorithms); - } - if (AST_VECTOR_INIT(algorithms, 4)) { - return -1; - } - while ((val.ptr = ast_strip(strsep(&iana_names, ",")))) { const pjsip_auth_algorithm *algo; @@ -719,7 +715,6 @@ static struct ast_cli_entry cli_commands[] = { static struct ast_sip_cli_formatter_entry *cli_formatter; -#if 1 static void global_loaded(const char *object_type) { ast_sorcery_force_reload_object(ast_sip_get_sorcery(), "auth"); @@ -729,7 +724,6 @@ static void global_loaded(const char *object_type) static struct ast_sorcery_observer global_observer = { .loaded = global_loaded, }; -#endif /*! \brief Initialize sorcery with auth support */ int ast_sip_initialize_sorcery_auth(void) diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c index 17b3f0f8e0..b2a7c69b3c 100644 --- a/res/res_pjsip/config_global.c +++ b/res/res_pjsip/config_global.c @@ -219,7 +219,7 @@ static int global_apply(const struct ast_sorcery *sorcery, void *obj) return -1; } - AST_VECTOR_INIT(&algorithms, 4); + AST_VECTOR_INIT(&algorithms, 0); res = ast_sip_auth_digest_algorithms_vector_init("global", &algorithms, "UAS", cfg->default_auth_algorithms_uas); AST_VECTOR_FREE(&algorithms); @@ -228,7 +228,7 @@ static int global_apply(const struct ast_sorcery *sorcery, void *obj) "Defaulting to %s\n", DEFAULT_AUTH_ALGORITHMS_UAS); ast_string_field_set(cfg, default_auth_algorithms_uas, DEFAULT_AUTH_ALGORITHMS_UAS); } - AST_VECTOR_INIT(&algorithms, 4); + AST_VECTOR_INIT(&algorithms, 0); res = ast_sip_auth_digest_algorithms_vector_init("global", &algorithms, "UAC", cfg->default_auth_algorithms_uac); AST_VECTOR_FREE(&algorithms); diff --git a/res/res_pjsip/pjsip_distributor.c b/res/res_pjsip/pjsip_distributor.c index 50608679cb..366ee9276c 100644 --- a/res/res_pjsip/pjsip_distributor.c +++ b/res/res_pjsip/pjsip_distributor.c @@ -616,13 +616,12 @@ static struct ast_sip_auth *alloc_artificial_auth(char *default_realm, static AO2_GLOBAL_OBJ_STATIC(artificial_auth); -static int create_artificial_auth(int reload) +static int create_artificial_auth(void) { char default_realm[AST_SIP_AUTH_MAX_REALM_LENGTH + 1]; struct ast_sip_auth *fake_auth; char default_algos_uac[AST_SIP_AUTH_MAX_SUPPORTED_ALGORITHMS_LENGTH + 1]; char default_algos_uas[AST_SIP_AUTH_MAX_SUPPORTED_ALGORITHMS_LENGTH + 1]; - int need_update = 1; ast_sip_get_default_realm(default_realm, sizeof(default_realm)); ast_sip_get_default_auth_algorithms_uac(default_algos_uac, @@ -630,34 +629,14 @@ static int create_artificial_auth(int reload) ast_sip_get_default_auth_algorithms_uas(default_algos_uas, sizeof(default_algos_uas)); - fake_auth = ast_sip_get_artificial_auth(); - if (fake_auth && reload) { - char *fake_algorithms_uac = NULL; - char *fake_algorithms_uas = NULL; - - ast_sip_auth_digest_algorithms_vector_to_str( - &fake_auth->supported_algorithms_uac, &fake_algorithms_uac); - ast_sip_auth_digest_algorithms_vector_to_str( - &fake_auth->supported_algorithms_uas, &fake_algorithms_uas); - if (strcmp(fake_auth->realm, default_realm) == 0 - && strcmp(fake_algorithms_uac, default_algos_uac) == 0 - && strcmp(fake_algorithms_uas, default_algos_uas) == 0) { - need_update = 0; - } - ast_free(fake_algorithms_uac); - ast_free(fake_algorithms_uas); - } - - ao2_cleanup(fake_auth); - if (!need_update) { - return 0; - } - fake_auth = alloc_artificial_auth(default_realm, default_algos_uac, default_algos_uas); - if (fake_auth) { - ao2_global_obj_replace_unref(artificial_auth, fake_auth); + if (!fake_auth) { + ast_log(LOG_ERROR, "Unable to create artificial auth\n"); + return -1; } + ao2_global_obj_replace_unref(artificial_auth, fake_auth); + ao2_cleanup(fake_auth); return 0; } @@ -1216,7 +1195,7 @@ static void global_loaded(const char *object_type) using_auth_username = new_using; } - create_artificial_auth(1); + create_artificial_auth(); ast_sip_get_unidentified_request_thresholds(&unidentified_count, &unidentified_period, &unidentified_prune_interval); @@ -1310,7 +1289,7 @@ int ast_sip_initialize_distributor(void) ast_sorcery_observer_add(ast_sip_get_sorcery(), "global", &global_observer); ast_sorcery_reload_object(ast_sip_get_sorcery(), "global"); - if (create_artificial_endpoint() || create_artificial_auth(0)) { + if (create_artificial_endpoint() || create_artificial_auth()) { ast_sip_destroy_distributor(); return -1; }