Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tdata leak when pjsip_inv_initial_answer() return error #3741

Merged
merged 4 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pjsip/include/pjsip-ua/sip_inv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • while here, perhaps add all params docs too?
  • adding a bit background about why p_tdata is set upon failure sounds nice? For example 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.

*/
PJ_DECL(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv,
pjsip_rx_data *rdata,
Expand Down
3 changes: 2 additions & 1 deletion pjsip/include/pjsip/sip_dialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
14 changes: 12 additions & 2 deletions pjsip/src/pjsip-ua/sip_inv.c
Original file line number Diff line number Diff line change
Expand Up @@ -2669,10 +2669,16 @@ PJ_DEF(pj_status_t) pjsip_inv_initial_answer( pjsip_inv_session *inv,
goto on_return;
}
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;
}
Expand Down Expand Up @@ -2755,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;
}
Expand Down
3 changes: 3 additions & 0 deletions pjsip/src/pjsua-lib/pjsua_call.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading