[Asterisk-code-review] res_pjsip: Prevent SEGV in pjsip_evsub_send_request (asterisk[20])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19901 )

Change subject: res_pjsip: Prevent SEGV in pjsip_evsub_send_request
......................................................................

res_pjsip: Prevent SEGV in pjsip_evsub_send_request

contributed pjproject - patch to check sub->pending_notify
in evsub.c:on_tsx_state before calling
pjsip_evsub_send_request()

res_pjsip_pubsub - change post pjsip 2.13 behavior to use
pubsub_on_refresh_timeout to avoid the ao2_cleanup call on
the sub_tree. This is is because the final NOTIFY send is no
longer the last place the sub_tree is referenced.

ASTERISK-30419

Change-Id: Ib5cc662ce578e9adcda312e16c58a10b6453e438
---
M res/res_pjsip_pubsub.c
A third-party/pjproject/patches/0010-Make-sure-that-NOTIFY-tdata-is-set-before-sending-it_new-129fb323a66dd1fd16880fe5ba5e6a57.patch
2 files changed, 67 insertions(+), 1 deletion(-)

Approvals:
  N A: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index c80e267..bf07d5a 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -4137,7 +4137,7 @@
 	   SUBSCRIBE's 200 OK. The previous solution was to push the NOTIFY, but now pjproject
 	   looks for the NOTIFY on send and delays it until after it auto-replies.
 	   If the NOTIFY is not there when it looks to send, pjproject will assert. */
-	serialized_pubsub_on_refresh_timeout(sub_tree);
+	pubsub_on_refresh_timeout(sub_tree);
 #else
 	if (ast_sip_push_task(sub_tree->serializer, serialized_pubsub_on_refresh_timeout, ao2_bump(sub_tree))) {
 		/* If we can't push the NOTIFY refreshing task...we'll just go with it. */
diff --git a/third-party/pjproject/patches/0010-Make-sure-that-NOTIFY-tdata-is-set-before-sending-it_new-129fb323a66dd1fd16880fe5ba5e6a57.patch b/third-party/pjproject/patches/0010-Make-sure-that-NOTIFY-tdata-is-set-before-sending-it_new-129fb323a66dd1fd16880fe5ba5e6a57.patch
new file mode 100644
index 0000000..009060a
--- /dev/null
+++ b/third-party/pjproject/patches/0010-Make-sure-that-NOTIFY-tdata-is-set-before-sending-it_new-129fb323a66dd1fd16880fe5ba5e6a57.patch
@@ -0,0 +1,46 @@
+From ac685b30c17be461b2bf5b46a772ed9742b8e985 Mon Sep 17 00:00:00 2001
+From: Riza Sulistyo <trengginas at users.noreply.github.com>
+Date: Thu, 9 Feb 2023 13:19:23 +0700
+Subject: [PATCH] Make sure that NOTIFY tdata is set before sending it.
+
+---
+ pjsip/src/pjsip-simple/evsub.c | 9 ++++++---
+ 1 file changed, 6 insertions(+), 3 deletions(-)
+
+diff --git a/pjsip/src/pjsip-simple/evsub.c b/pjsip/src/pjsip-simple/evsub.c
+index da0a9b416..68c1d3951 100644
+--- a/pjsip/src/pjsip-simple/evsub.c
++++ b/pjsip/src/pjsip-simple/evsub.c
+@@ -2216,23 +2216,26 @@ static void on_tsx_state_uas( pjsip_evsub *sub, pjsip_transaction *tsx,
+             }
+ 
+         }  else {
+             sub->state = old_state;
+             sub->state_str = old_state_str;
+         }
+ 
+         /* Send the pending NOTIFY sent by app from inside
+          * on_rx_refresh() callback.
+          */
+-        pj_assert(sub->pending_notify);
+-        status = pjsip_evsub_send_request(sub, sub->pending_notify);
+-        sub->pending_notify = NULL;
++        //pj_assert(sub->pending_notify);
++        /* Make sure that pending_notify is set. */
++        if (sub->pending_notify) {
++            status = pjsip_evsub_send_request(sub, sub->pending_notify);
++            sub->pending_notify = NULL;
++        }
+ 
+     } else if (pjsip_method_cmp(&tsx->method, &pjsip_notify_method)==0) {
+ 
+         /* Handle authentication */
+         if (tsx->state == PJSIP_TSX_STATE_COMPLETED &&
+             (tsx->status_code==401 || tsx->status_code==407))
+         {
+             pjsip_tx_data *tdata;
+             pj_status_t status;
+             pjsip_rx_data *rdata = event->body.tsx_state.src.rdata;
+-- 
+2.39.1
+

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19901
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 20
Gerrit-Change-Id: Ib5cc662ce578e9adcda312e16c58a10b6453e438
Gerrit-Change-Number: 19901
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: N A <asterisk at phreaknet.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20230227/4a330dca/attachment-0001.html>