fixeria has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-msc/+/14500


Change subject: libmsc/gsm_09_11.c: fix broken reference counting for vsub
......................................................................

libmsc/gsm_09_11.c: fix broken reference counting for vsub

In gsm0911_gsup_rx() we do call vlr_subscr_find_by_imsi(), which
increases subscriber's reference count by one using the function
name as the token. However, we never release this token, so the
reference count grows on every received GSUP PROC-SS message.

Change-Id: I5540556b1c75f6873883e46b78656f31fc1ef186
---
M src/libmsc/gsm_09_11.c
M tests/msc_vlr/msc_vlr_test_ss.err
2 files changed, 36 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/00/14500/1

diff --git a/src/libmsc/gsm_09_11.c b/src/libmsc/gsm_09_11.c
index cd54703..c7b2155 100644
--- a/src/libmsc/gsm_09_11.c
+++ b/src/libmsc/gsm_09_11.c
@@ -424,8 +424,9 @@
        struct msgb *ss_msg;
        bool trans_end;
        struct msc_a *msc_a;
-       struct vlr_subscr *vsub = vlr_subscr_find_by_imsi(net->vlr, 
gsup_msg->imsi, __func__);
+       struct vlr_subscr *vsub;

+       vsub = vlr_subscr_find_by_imsi(net->vlr, gsup_msg->imsi, __func__);
        if (!vsub) {
                LOGP(DSS, LOGL_ERROR, "Rx %s for unknown subscriber, 
rejecting\n",
                     osmo_gsup_message_type_name(gsup_msg->message_type));
@@ -445,6 +446,9 @@
                     osmo_gsup_message_type_name(gsup_msg->message_type),
                     gsup_msg->cause, gsup_msg->session_id);

+               /* We don't need subscriber info anymore */
+               vlr_subscr_put(vsub, __func__);
+
                if (!trans) {
                        LOGP(DSS, LOGL_ERROR, "No transaction found for "
                             "sid=0x%x, nothing to abort\n", 
gsup_msg->session_id);
@@ -477,14 +481,20 @@
                                              "SS/USSD transaction, rejecting 
%s\n",
                                              
osmo_gsup_message_type_name(gsup_msg->message_type));
                        gsup_client_mux_tx_error_reply(gcm, gsup_msg, 
GMM_CAUSE_NET_FAIL);
+                       vlr_subscr_put(vsub, __func__);
                        return -EINVAL;
                }

                /* Wait for Paging Response */
-               if (trans->paging_request)
+               if (trans->paging_request) {
+                       vlr_subscr_put(vsub, __func__);
                        return 0;
+               }
        }

+       /* We don't need subscriber info anymore */
+       vlr_subscr_put(vsub, __func__);
+
        /* (Re)schedule the inactivity timer */
        if (net->ncss_guard_timeout > 0) {
                osmo_timer_schedule(&trans->ss.timer_guard,
diff --git a/tests/msc_vlr/msc_vlr_test_ss.err 
b/tests/msc_vlr/msc_vlr_test_ss.err
index 7845c99..9f47dba 100644
--- a/tests/msc_vlr/msc_vlr_test_ss.err
+++ b/tests/msc_vlr/msc_vlr_test_ss.err
@@ -184,23 +184,24 @@
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}:
 - rx_from_ms: now used by 1 (nc_ss)
 <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: 
20010809710000004026f03004200000013101033527a225020101302002013b301b04010f0416d9775d0e2ae3e965f73cfd7683d27310cd06bbc51a0d0a0103
 DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + gsm0911_gsup_rx: now used 
by 4 (attached,active-conn,NCSS,gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - gsm0911_gsup_rx: now used 
by 3 (attached,active-conn,NCSS)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}:
 RAN encode: DTAP on GERAN-A
 - DTAP --GERAN-A--> MS: GSM0480_MTYPE_RELEASE_COMPLETE: 
8b2a1c27a225020101302002013b301b04010f0416d9775d0e2ae3e965f73cfd7683d27310cd06bbc51a0d
 - DTAP matches expected message
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){0}: 
Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
 DSS trans(NCSS IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ 
callref-0x20000001 tid-8) Freeing transaction
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - NCSS: now used by 3 
(attached,active-conn,gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - NCSS: now used by 2 
(attached,active-conn)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}:
 - nc_ss: now used by 0 (-)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}:
 Received Event MSC_A_EV_UNUSED
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_COMMUNICATING}:
 state_chg to MSC_A_ST_RELEASING
 DBSSAP 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_RELEASING}:
 Releasing: msc_a use is 0 (-)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
msc_a_fsm_releasing_onenter: now used by 4 
(attached,active-conn,gsm0911_gsup_rx,msc_a_fsm_releasing_onenter)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
vlr_subscr_cancel_attach_fsm: now used by 5 
(attached,active-conn,gsm0911_gsup_rx,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
vlr_subscr_cancel_attach_fsm: now used by 4 
(attached,active-conn,gsm0911_gsup_rx,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
msc_a_fsm_releasing_onenter: now used by 3 
(attached,active-conn,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
vlr_subscr_cancel_attach_fsm: now used by 4 
(attached,active-conn,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
vlr_subscr_cancel_attach_fsm: now used by 3 
(attached,active-conn,msc_a_fsm_releasing_onenter)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_RELEASING}:
 + wait-Clear-Complete: now used by 1 (wait-Clear-Complete)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_RELEASING}:
 RAN encode: CLEAR_COMMAND on GERAN-A
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){0}: 
Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 3 
(attached,active-conn,gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 2 (attached,active-conn)
 <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: vlr_gsupc_read_cb() returns 0
   dtap_tx_confirmed == 1
   bssap_clear_sent == 1
@@ -225,7 +226,7 @@
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){0}: 
Removing from parent msub_fsm
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){0}: 
Deferring: will deallocate with 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ)
 DMSC msub(IMSI-901700000004620:MSISDN-46071) Free
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - active-conn: now used by 2 
(attached,gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - active-conn: now used by 1 
(attached)
 DMSC msub_fsm{terminating}: Deferring: will deallocate with 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_RELEASED}:
 Deallocated, including all deferred deallocations
 - msub gone
@@ -375,14 +376,15 @@
   paging request (SIGNALLING_HIGH_PRIO) to IMSI-901700000004620:MSISDN-46071 
on GERAN-A
   strcmp(paging_expecting_imsi, vsub->imsi) == 0
 DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + Paging: now used by 5 
(attached,_test_ss_ussd_no,gsm0911_gsup_rx,NCSS,Paging)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - gsm0911_gsup_rx: now used 
by 4 (attached,_test_ss_ussd_no,NCSS,Paging)
 <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: vlr_gsupc_read_cb() returns 0
   llist_count(&vsub->cs.requests) == 1
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - _test_ss_ussd_no: now used 
by 4 (attached,gsm0911_gsup_rx,NCSS,Paging)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - _test_ss_ussd_no: now used 
by 3 (attached,NCSS,Paging)
   paging_sent == 1
 - the subscriber and its pending request should remain
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + _test_ss_ussd_no: now used 
by 5 (attached,gsm0911_gsup_rx,NCSS,Paging,_test_ss_ussd_no)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + _test_ss_ussd_no: now used 
by 4 (attached,NCSS,Paging,_test_ss_ussd_no)
   llist_count(&vsub->cs.requests) == 1
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - _test_ss_ussd_no: now used 
by 4 (attached,gsm0911_gsup_rx,NCSS,Paging)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - _test_ss_ussd_no: now used 
by 3 (attached,NCSS,Paging)
 - MS replies with Paging Response, we deliver the NC/USSD
   MSC <--GERAN-A-- MS: GSM48_MT_RR_PAG_RESP
   new conn
@@ -401,8 +403,8 @@
 DVLR 
Process_Access_Request_VLR(IMSI-901700000004620:GERAN-A:PAGING_RESP){PR_ARQ_S_INIT}:
 is child of msc_a(IMSI-901700000004620:GERAN-A:PAGING_RESP)
 DVLR 
Process_Access_Request_VLR(IMSI-901700000004620:GERAN-A:PAGING_RESP){PR_ARQ_S_INIT}:
 rev=GSM net=GERAN (no Auth)
 DVLR 
Process_Access_Request_VLR(IMSI-901700000004620:GERAN-A:PAGING_RESP){PR_ARQ_S_INIT}:
 Received Event PR_ARQ_E_START
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + proc_arq_vlr_fn_init: now 
used by 5 (attached,gsm0911_gsup_rx,NCSS,Paging,proc_arq_vlr_fn_init)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + active-conn: now used by 6 
(attached,gsm0911_gsup_rx,NCSS,Paging,proc_arq_vlr_fn_init,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + proc_arq_vlr_fn_init: now 
used by 4 (attached,NCSS,Paging,proc_arq_vlr_fn_init)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + active-conn: now used by 5 
(attached,NCSS,Paging,proc_arq_vlr_fn_init,active-conn)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_VALIDATE_L3}:
 Received Event MSC_A_EV_COMPLETE_LAYER_3_OK
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_VALIDATE_L3}:
 state_chg to MSC_A_ST_AUTH_CIPH
 DVLR 
Process_Access_Request_VLR(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){PR_ARQ_S_INIT}:
 proc_arq_vlr_fn_post_imsi()
@@ -425,9 +427,9 @@
 - DTAP --GERAN-A--> MS: GSM0480_MTYPE_REGISTER: 
0b3b1c15a11302010102013b300b04010f0406aa510c061b01
 - DTAP matches expected message
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){0}: 
Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - Paging: now used by 5 
(attached,gsm0911_gsup_rx,NCSS,proc_arq_vlr_fn_init,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - Paging: now used by 4 
(attached,NCSS,proc_arq_vlr_fn_init,active-conn)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 - paging-response: now used by 2 (rx_from_ms,nc_ss)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - proc_arq_vlr_fn_init: now 
used by 4 (attached,gsm0911_gsup_rx,NCSS,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - proc_arq_vlr_fn_init: now 
used by 3 (attached,NCSS,active-conn)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 - rx_from_ms: now used by 1 (nc_ss)
   dtap_tx_confirmed == 1
 - MS responds to SS/USSD request
@@ -441,24 +443,25 @@
   dtap_tx_confirmed == 1
 - HLR terminates the session
 <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: 
20010809710000004026f03004200001013101030a0103
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + gsm0911_gsup_rx: now used 
by 5 (attached,2*gsm0911_gsup_rx,NCSS,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + gsm0911_gsup_rx: now used 
by 4 (attached,NCSS,active-conn,gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - gsm0911_gsup_rx: now used 
by 3 (attached,NCSS,active-conn)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 RAN encode: DTAP on GERAN-A
 - DTAP --GERAN-A--> MS: GSM0480_MTYPE_RELEASE_COMPLETE: 0b2a
 - DTAP matches expected message
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){0}: 
Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
 DSS trans(NCSS IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP 
callref-0x20000101 tid-0) Freeing transaction
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - NCSS: now used by 4 
(attached,2*gsm0911_gsup_rx,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - NCSS: now used by 2 
(attached,active-conn)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 - nc_ss: now used by 0 (-)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 Received Event MSC_A_EV_UNUSED
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}:
 state_chg to MSC_A_ST_RELEASING
 DBSSAP 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_RELEASING}:
 Releasing: msc_a use is 0 (-)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
msc_a_fsm_releasing_onenter: now used by 5 
(attached,2*gsm0911_gsup_rx,active-conn,msc_a_fsm_releasing_onenter)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
vlr_subscr_cancel_attach_fsm: now used by 6 
(attached,2*gsm0911_gsup_rx,active-conn,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
vlr_subscr_cancel_attach_fsm: now used by 5 
(attached,2*gsm0911_gsup_rx,active-conn,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
msc_a_fsm_releasing_onenter: now used by 3 
(attached,active-conn,msc_a_fsm_releasing_onenter)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 + 
vlr_subscr_cancel_attach_fsm: now used by 4 
(attached,active-conn,msc_a_fsm_releasing_onenter,vlr_subscr_cancel_attach_fsm)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
vlr_subscr_cancel_attach_fsm: now used by 3 
(attached,active-conn,msc_a_fsm_releasing_onenter)
 DREF 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_RELEASING}:
 + wait-Clear-Complete: now used by 1 (wait-Clear-Complete)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_RELEASING}:
 RAN encode: CLEAR_COMMAND on GERAN-A
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){0}: 
Received Event MSC_I_EV_FROM_A_FORWARD_ACCESS_SIGNALLING_REQUEST
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 4 
(attached,2*gsm0911_gsup_rx,active-conn)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - 
msc_a_fsm_releasing_onenter: now used by 2 (attached,active-conn)
 <-- GSUP rx OSMO_GSUP_MSGT_PROC_SS_REQUEST: vlr_gsupc_read_cb() returns 0
   dtap_tx_confirmed == 1
   bssap_clear_sent == 1
@@ -483,12 +486,12 @@
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){0}: 
Removing from parent msub_fsm
 DMSC dummy_msc_i(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){0}: 
Deferring: will deallocate with 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP)
 DMSC msub(IMSI-901700000004620:MSISDN-46071) Free
-DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - active-conn: now used by 3 
(attached,2*gsm0911_gsup_rx)
+DREF VLR subscr IMSI-901700000004620:MSISDN-46071 - active-conn: now used by 1 
(attached)
 DMSC msub_fsm{terminating}: Deferring: will deallocate with 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP)
 DMSC 
msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_RELEASED}:
 Deallocated, including all deferred deallocations
 - msub gone
   llist_count(&msub_list) == 0
-DVLR freeing VLR subscr IMSI-901700000004620:MSISDN-46071 (max total use count 
was 6)
+DVLR freeing VLR subscr IMSI-901700000004620:MSISDN-46071 (max total use count 
was 5)
 ===== test_ss_ussd_no_geran: SUCCESS

 full talloc report on 'msgb' (total      0 bytes in   1 blocks)

--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/14500
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I5540556b1c75f6873883e46b78656f31fc1ef186
Gerrit-Change-Number: 14500
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <axilira...@gmail.com>
Gerrit-MessageType: newchange

Reply via email to