From 43dd94f4ec257993959f381ea46091ece7aaf85d Mon Sep 17 00:00:00 2001 From: Riza Sulistyo Date: Tue, 17 Oct 2023 21:52:27 +0700 Subject: [PATCH 1/4] Fix tdata leak when pjsip_inv_initial_answer() return error --- pjsip/src/pjsip-ua/sip_inv.c | 2 +- pjsip/src/pjsua-lib/pjsua_call.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c index 0c83426071..b6d6b5c9dd 100644 --- a/pjsip/src/pjsip-ua/sip_inv.c +++ b/pjsip/src/pjsip-ua/sip_inv.c @@ -2664,8 +2664,8 @@ PJ_DEF(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv, pj_status_t status2; status2 = pjsip_dlg_modify_response(inv->dlg, tdata, st_code2, NULL); + pjsip_tx_data_dec_ref(tdata); if (status2 != PJ_SUCCESS) { - pjsip_tx_data_dec_ref(tdata); goto on_return; } status2 = pjsip_timer_update_resp(inv, tdata); diff --git a/pjsip/src/pjsua-lib/pjsua_call.c b/pjsip/src/pjsua-lib/pjsua_call.c index 9cb7ae37eb..0a78d0f963 100644 --- a/pjsip/src/pjsua-lib/pjsua_call.c +++ b/pjsip/src/pjsua-lib/pjsua_call.c @@ -5552,6 +5552,9 @@ static void pjsua_call_on_rx_offer(pjsip_inv_session *inv, (pjsip_rx_data *)param->rdata, 100, NULL, NULL, &response); if (status != PJ_SUCCESS) { + if (response) { + pjsip_tx_data_dec_ref(response); + } PJ_PERROR(3, (THIS_FILE, status, "Failed to create initial answer")); goto on_return; From d013d264e782f1d1543ba7dd6194208bf888ddc6 Mon Sep 17 00:00:00 2001 From: Riza Sulistyo Date: Wed, 18 Oct 2023 10:49:39 +0700 Subject: [PATCH 2/4] Set the inv->last_answer with tdata --- pjsip/src/pjsip-ua/sip_inv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c index b6d6b5c9dd..e642d98f07 100644 --- a/pjsip/src/pjsip-ua/sip_inv.c +++ b/pjsip/src/pjsip-ua/sip_inv.c @@ -2664,10 +2664,11 @@ PJ_DEF(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv, pj_status_t status2; status2 = pjsip_dlg_modify_response(inv->dlg, tdata, st_code2, NULL); - pjsip_tx_data_dec_ref(tdata); if (status2 != PJ_SUCCESS) { + pjsip_tx_data_dec_ref(tdata); goto on_return; } + inv->last_answer = tdata; status2 = pjsip_timer_update_resp(inv, tdata); if (status2 == PJ_SUCCESS) *p_tdata = tdata; From 03c8a8acee1e71b5812b8e6af3843a7451c0baec Mon Sep 17 00:00:00 2001 From: Riza Sulistyo Date: Thu, 19 Oct 2023 09:58:37 +0700 Subject: [PATCH 3/4] Modification based on comments - Add doc - Add pjsip_tx_data_dec_ref() on failure case --- pjsip/include/pjsip-ua/sip_inv.h | 4 ++++ pjsip/include/pjsip/sip_dialog.h | 3 ++- pjsip/src/pjsip-ua/sip_inv.c | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/pjsip/include/pjsip-ua/sip_inv.h b/pjsip/include/pjsip-ua/sip_inv.h index c791ccc9cd..0c6ebe7994 100644 --- a/pjsip/include/pjsip-ua/sip_inv.h +++ b/pjsip/include/pjsip-ua/sip_inv.h @@ -810,6 +810,10 @@ PJ_DECL(pj_status_t) pjsip_inv_invite( pjsip_inv_session *inv, * Create the initial response message for the incoming INVITE request in * rdata with status code st_code and optional status text st_text. Use * #pjsip_inv_answer() to create subsequent response message. + * Notes: + * - Upon failure, p_tdata might still get set. To avoid leak, + * it needs to be sent or otherwise decrement the reference count using + * #pjsip_tx_data_dec_ref(). */ PJ_DECL(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv, pjsip_rx_data *rdata, diff --git a/pjsip/include/pjsip/sip_dialog.h b/pjsip/include/pjsip/sip_dialog.h index dd842677e3..4833847546 100644 --- a/pjsip/include/pjsip/sip_dialog.h +++ b/pjsip/include/pjsip/sip_dialog.h @@ -740,7 +740,8 @@ PJ_DECL(pj_status_t) pjsip_dlg_create_response( pjsip_dialog *dlg, * * @param dlg The dialog. * @param tdata The transmit data buffer containing response - * message to be modified. + * message to be modified. Upon successful return, + * the reference count will be incremented. * @param st_code New status code to be set. * @param st_text Optional string for custom status reason text. * diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c index e642d98f07..fb4ce71123 100644 --- a/pjsip/src/pjsip-ua/sip_inv.c +++ b/pjsip/src/pjsip-ua/sip_inv.c @@ -2668,12 +2668,17 @@ PJ_DEF(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv, pjsip_tx_data_dec_ref(tdata); goto on_return; } - inv->last_answer = tdata; status2 = pjsip_timer_update_resp(inv, tdata); - if (status2 == PJ_SUCCESS) + if (status2 == PJ_SUCCESS) { + inv->last_answer = tdata; *p_tdata = tdata; - else + } else { + /* To avoid leak, we need to decrement 2 times since + * pjsip_dlg_modify_response() increment tdata ref count. + */ + pjsip_tx_data_dec_ref(tdata); pjsip_tx_data_dec_ref(tdata); + } goto on_return; } @@ -2756,6 +2761,10 @@ PJ_DEF(pj_status_t) pjsip_inv_answer( pjsip_inv_session *inv, /* Process SDP in answer */ status = process_answer(inv, st_code, last_res, local_sdp); if (status != PJ_SUCCESS) { + /* To avoid leak, we need to decrement 2 times since + * pjsip_dlg_modify_response() increment tdata ref count. + */ + pjsip_tx_data_dec_ref(last_res); pjsip_tx_data_dec_ref(last_res); goto on_return; } From c979e5085f2bd66ea415e8da55240444dd765379 Mon Sep 17 00:00:00 2001 From: Riza Sulistyo Date: Thu, 19 Oct 2023 10:50:00 +0700 Subject: [PATCH 4/4] Modify doc --- pjsip/include/pjsip-ua/sip_inv.h | 37 +++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/pjsip/include/pjsip-ua/sip_inv.h b/pjsip/include/pjsip-ua/sip_inv.h index 0c6ebe7994..bb0b1ff020 100644 --- a/pjsip/include/pjsip-ua/sip_inv.h +++ b/pjsip/include/pjsip-ua/sip_inv.h @@ -808,12 +808,39 @@ PJ_DECL(pj_status_t) pjsip_inv_invite( pjsip_inv_session *inv, /** * Create the initial response message for the incoming INVITE request in - * rdata with status code st_code and optional status text st_text. Use + * rdata with status code st_code and optional status text st_text. Use * #pjsip_inv_answer() to create subsequent response message. - * Notes: - * - Upon failure, p_tdata might still get set. To avoid leak, - * it needs to be sent or otherwise decrement the reference count using - * #pjsip_tx_data_dec_ref(). + * + * When this function returning non-PJ_SUCCESS, it may be caused by an + * unacceptable INVITE request. In such cases the function will generate + * an appropriate answer in p_tdata, + * e.g: when session timer header Session-Expires is too low, + * the generated answer will include Min-SE header. + * If the generated answer is not sent, it must be destroyed. + * i.e: using #pjsip_tx_data_dec_ref(), to avoid resource leak. + * + * @param inv The UAS invite session. + * @param rdata The incoming request message. + * @param st_code The st_code contains the status code to be sent, + * which may be a provisional or final response. + * @param st_text If custom status text is desired, application can + * specify the text in st_text; otherwise if this + * argument is NULL, default status text will be used. + * @param sdp If application has specified its media capability + * during creation of UAS invite session, the sdp + * argument MUST be NULL. This is because application + * can not perform more than one SDP offer/answer session + * in a single INVITE transaction. + * If application has not specified its media capability + * during creation of UAS invite session, it MAY or MUST + * specify its capability in sdp argument, + * depending whether st_code indicates a 2xx final + * response. + * @param p_tdata Pointer to receive the response message created by + * this function. + * + * @return PJ_SUCCESS if response message was created + * successfully. */ PJ_DECL(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv, pjsip_rx_data *rdata,