fix crash on CM Serv Rej: fix use count mismatch

With comments, clarify the code paths where a CM Service use count has
not yet been placed on the conn (just send CM Service Reject) and where
the use count is placed (decrement count on CM Service Reject).

Place the CM Service use count slightly earlier:
- it is then correctly present when checking the mobile identity in
  cm_serv_reuse_conn(), avoiding the crash reported in OS#5532.
- there is only one place incrementing the use count instead of two.

Related: OS#5532
Change-Id: I6c735b79b67108bcaadada3f01c7046e262f939b
This commit is contained in:
Neels Hofmeyr 2022-04-24 23:37:07 +02:00
parent c628c9e256
commit 5d53c6001d
3 changed files with 24 additions and 7 deletions

View File

@ -246,7 +246,9 @@ static int msc_gsm48_tx_mm_serv_ack(struct msc_a *msc_a)
return msc_a_tx_dtap_to_i(msc_a, msg); return msc_a_tx_dtap_to_i(msc_a, msg);
} }
/* 9.2.6 CM service reject */ /* 9.2.6 CM service reject.
* For an active and valid CM Service Request, instead use msc_vlr_tx_cm_serv_rej(), which also takes care of
* decrementing the use token for that service type. */
static int msc_gsm48_tx_mm_serv_rej(struct msc_a *msc_a, static int msc_gsm48_tx_mm_serv_rej(struct msc_a *msc_a,
enum gsm48_reject_value value) enum gsm48_reject_value value)
{ {
@ -701,7 +703,6 @@ static int cm_serv_reuse_conn(struct msc_a *msc_a, const struct osmo_mobile_iden
accept_reuse: accept_reuse:
LOG_MSC_A_CAT(msc_a, DMM, LOGL_DEBUG, "re-using already accepted connection\n"); LOG_MSC_A_CAT(msc_a, DMM, LOGL_DEBUG, "re-using already accepted connection\n");
msc_a_get(msc_a, msc_a_cm_service_type_to_use(cm_serv_type));
msub_update_id(msc_a->c.msub); msub_update_id(msc_a->c.msub);
return net->vlr->ops.tx_cm_serv_acc(msc_a, cm_serv_type); return net->vlr->ops.tx_cm_serv_acc(msc_a, cm_serv_type);
} }
@ -729,6 +730,14 @@ int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
struct osmo_mobile_identity mi; struct osmo_mobile_identity mi;
int rc; int rc;
/* There are two ways to respond with a CM Service Reject:
* Directly and only send the CM Service Reject with msc_gsm48_tx_mm_serv_rej().
* Decrement the CM Service use count token and send the message with msc_vlr_tx_cm_serv_rej().
*
* Until we accept the CM Service Request message as such, there is no use count placed for the service type.
* So in here use msc_gsm48_tx_mm_serv_rej() to respond.
*/
/* Make sure that both header and CM Service Request fit into the buffer */ /* Make sure that both header and CM Service Request fit into the buffer */
if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*req)) { if (msgb_l3len(msg) < sizeof(*gh) + sizeof(*req)) {
LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: wrong message size (%u < %zu)\n", LOG_MSC_A(msc_a, LOGL_ERROR, "Rx CM SERVICE REQUEST: wrong message size (%u < %zu)\n",
@ -791,13 +800,16 @@ int gsm48_rx_mm_serv_req(struct msc_a *msc_a, struct msgb *msg)
if (!msc_a_cm_service_type_to_use(req->cm_service_type)) if (!msc_a_cm_service_type_to_use(req->cm_service_type))
return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_SRV_OPT_NOT_SUPPORTED); return msc_gsm48_tx_mm_serv_rej(msc_a, GSM48_REJECT_SRV_OPT_NOT_SUPPORTED);
/* At this point, the CM Service Request message is being accepted.
* Increment the matching use token, and from here on use msc_vlr_tx_cm_serv_rej() to respond in case of
* failure. */
msc_a_get(msc_a, msc_a_cm_service_type_to_use(req->cm_service_type));
if (msc_a_is_accepted(msc_a)) if (msc_a_is_accepted(msc_a))
return cm_serv_reuse_conn(msc_a, &mi, req->cm_service_type); return cm_serv_reuse_conn(msc_a, &mi, req->cm_service_type);
osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, &mi); osmo_signal_dispatch(SS_SUBSCR, S_SUBSCR_IDENTITY, &mi);
msc_a_get(msc_a, msc_a_cm_service_type_to_use(req->cm_service_type));
is_utran = (msc_a->c.ran->type == OSMO_RAT_UTRAN_IU); is_utran = (msc_a->c.ran->type == OSMO_RAT_UTRAN_IU);
vlr_proc_acc_req(msc_a->c.fi, vlr_proc_acc_req(msc_a->c.fi,
MSC_A_EV_AUTHENTICATED, MSC_A_EV_CN_CLOSE, NULL, MSC_A_EV_AUTHENTICATED, MSC_A_EV_CN_CLOSE, NULL,
@ -1508,7 +1520,8 @@ static int msc_vlr_tx_mm_info(void *msc_conn_ref)
return gsm48_tx_mm_info(msc_a); return gsm48_tx_mm_info(msc_a);
} }
/* VLR asks us to transmit a CM Service Reject */ /* VLR asks us to transmit a CM Service Reject.
* Decrement the CM Service type's use token and send the CM Service Reject message. */
static int msc_vlr_tx_cm_serv_rej(void *msc_conn_ref, enum osmo_cm_service_type cm_service_type, static int msc_vlr_tx_cm_serv_rej(void *msc_conn_ref, enum osmo_cm_service_type cm_service_type,
enum gsm48_reject_value cause) enum gsm48_reject_value cause)
{ {

View File

@ -1792,6 +1792,10 @@ int msc_a_try_call_assignment(struct gsm_trans *cc_trans)
return msc_a_start_assignment(msc_a, cc_trans); return msc_a_start_assignment(msc_a, cc_trans);
} }
/* Map CM Service type to use token.
* Given a CM Service type, return a matching token intended for osmo_use_count.
* For unknown service type, return NULL.
*/
const char *msc_a_cm_service_type_to_use(enum osmo_cm_service_type cm_service_type) const char *msc_a_cm_service_type_to_use(enum osmo_cm_service_type cm_service_type)
{ {
switch (cm_service_type) { switch (cm_service_type) {

View File

@ -1032,8 +1032,8 @@ DREF msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AU
DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN decode: DTAP DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN decode: DTAP
DRLL msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Dispatching 04.08 message: MM GSM48_MT_MM_CM_SERV_REQ DRLL msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Dispatching 04.08 message: MM GSM48_MT_MM_CM_SERV_REQ
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Rx CM SERVICE REQUEST cm_service_type=Short-Messaging-Service DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Rx CM SERVICE REQUEST cm_service_type=Short-Messaging-Service
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: re-using already accepted connection
DREF msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: + cm_service_sms: now used by 3 (2*cm_service_sms,rx_from_ms) DREF msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: + cm_service_sms: now used by 3 (2*cm_service_sms,rx_from_ms)
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: re-using already accepted connection
DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Sending DTAP: MM GSM48_MT_MM_CM_SERV_ACC DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Sending DTAP: MM GSM48_MT_MM_CM_SERV_ACC
DMSC msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN encode: DTAP on GERAN-A DMSC msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN encode: DTAP on GERAN-A
- DTAP --GERAN-A--> MS: GSM48_MT_MM_CM_SERV_ACC: 0521 - DTAP --GERAN-A--> MS: GSM48_MT_MM_CM_SERV_ACC: 0521
@ -1842,8 +1842,8 @@ DREF msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHE
DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}: RAN decode: DTAP DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}: RAN decode: DTAP
DRLL msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}: Dispatching 04.08 message: MM GSM48_MT_MM_CM_SERV_REQ DRLL msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:PAGING_RESP){MSC_A_ST_AUTHENTICATED}: Dispatching 04.08 message: MM GSM48_MT_MM_CM_SERV_REQ
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Rx CM SERVICE REQUEST cm_service_type=Short-Messaging-Service DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Rx CM SERVICE REQUEST cm_service_type=Short-Messaging-Service
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: re-using already accepted connection
DREF msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: + cm_service_sms: now used by 3 (sms,rx_from_ms,cm_service_sms) DREF msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: + cm_service_sms: now used by 3 (sms,rx_from_ms,cm_service_sms)
DMM msc_a(IMSI-901700000004620:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: re-using already accepted connection
DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Sending DTAP: MM GSM48_MT_MM_CM_SERV_ACC DBSSAP msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: Sending DTAP: MM GSM48_MT_MM_CM_SERV_ACC
DMSC msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN encode: DTAP on GERAN-A DMSC msc_a(IMSI-901700000004620:MSISDN-46071:GERAN-A:CM_SERVICE_REQ){MSC_A_ST_AUTHENTICATED}: RAN encode: DTAP on GERAN-A
- DTAP --GERAN-A--> MS: GSM48_MT_MM_CM_SERV_ACC: 0521 - DTAP --GERAN-A--> MS: GSM48_MT_MM_CM_SERV_ACC: 0521