From 569b1eb34c268cd04585684fc69a63b70a3b9027 Mon Sep 17 00:00:00 2001 From: dondetir Date: Tue, 28 Apr 2026 18:37:21 -0500 Subject: [PATCH 1/2] aaa_diameter: always release reply cJSON regardless of output PV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _dm_release_message_response() is the only function that frees the SHM-backed cJSON reply tree (cond->rpl.json). In dm_send_request(), the release call was gated inside "if (rpl_avps_pv)", so every call without an output PV leaked the entire SHM cJSON tree. The same issue existed in dm_send_request_async_reply(): the release was guarded by "if (rpl_avps)", which was NULL both when no output PV was configured and on the read() failure path — even though the reply callback had already populated cond->rpl.json. _dm_release_message_response() safely handles a NULL rpl_avps argument (cJSON_PurgeString is a no-op on NULL), so removing both guards is safe. Found during a systematic audit of dm_send_request() paths following commit 1e8001fa0. --- modules/aaa_diameter/aaa_diameter.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/aaa_diameter/aaa_diameter.c b/modules/aaa_diameter/aaa_diameter.c index fa1933044f..3a426bd929 100644 --- a/modules/aaa_diameter/aaa_diameter.c +++ b/modules/aaa_diameter/aaa_diameter.c @@ -337,8 +337,8 @@ static int dm_send_request(struct sip_msg *msg, int *app_id, int *cmd_code, pv_value_t val = {(str){rpl_avps, strlen(rpl_avps)}, 0, PV_VAL_STR}; if (pv_set_value(msg, rpl_avps_pv, 0, &val) != 0) LM_ERR("failed to set output rpl_avps pv to: %s\n", rpl_avps); - _dm_release_message_response(rpl, rpl_avps); } + _dm_release_message_response(rpl, rpl_avps); if (rc != 0) { LM_ERR("Diameter request failed (rc: %d)\n", rc); @@ -521,8 +521,7 @@ static int dm_send_request_async_reply(int fd, error: if (amsg->ret && pv_set_value(msg, amsg->ret, 0, &val) != 0) LM_ERR("failed to set output rpl_avps pv to NULL\n"); - if (rpl_avps) - _dm_release_message_response(amsg->cond, rpl_avps); + _dm_release_message_response(amsg->cond, rpl_avps); dm_free_sync_msg(amsg); return ret; } From 77a23077e25b5a0c25d56752f3c3371ab43f9296 Mon Sep 17 00:00:00 2001 From: dondetir Date: Tue, 28 Apr 2026 18:37:26 -0500 Subject: [PATCH 2/2] aaa_diameter: fix double-free of cJSON item in dm_avps2json() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After cJSON_AddItemToArray(avps, item) transfers ownership of item to avps, item is not cleared. Any subsequent FD_CHECK_GT failure in the same or next loop iteration jumps to the out: label, which calls cJSON_Delete(item) — freeing memory already owned by avps. This double-free corrupts the SHM heap, affecting all worker processes. The affected paths include: - FD_CHECK_GT(fd_msg_browse()) at the skip: label (next-iteration advance) - FD_CHECK_GT(fd_msg_avp_hdr()) and FD_CHECK_GT(fd_dict_getval()) at the start of the next iteration Fix: clear item immediately after the ownership transfer so that any later cJSON_Delete(item) at out: is a safe no-op. Found during a systematic audit following commit 1e8001fa0. --- modules/aaa_diameter/dm_impl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/aaa_diameter/dm_impl.c b/modules/aaa_diameter/dm_impl.c index 7d22714623..252a523535 100644 --- a/modules/aaa_diameter/dm_impl.c +++ b/modules/aaa_diameter/dm_impl.c @@ -523,8 +523,9 @@ static int dm_avps2json(void *root, cJSON *avps) add: cJSON_AddItemToObject(item, dm_avp.avp_name, val); cJSON_AddItemToArray(avps, item); + item = NULL; -skip: +skip: FD_CHECK_GT(fd_msg_browse(it, MSG_BRW_NEXT, &it, NULL)); i++; }