Review at https://gerrit.osmocom.org/152
gprs_gmm.c: Don't try to de-reference NULL mmctx
There was a comment in the code that certain GMM messages require a
valid mmctx pointer. However, nothing actually checked if that pointer
was in fact non-NULL. We plainly crashed if a MS would send us the
wrong message in the wrong state.
Original patch by Harald Welte, but it broke message validity checking,
resulting in sgsn_test failure. This re-implements the NULL check in a
different way, as explained by in-code comment.
Change-Id: I10e6fee84abf05179f9e70981cdd295c57a58391
---
M openbsc/src/gprs/gprs_gmm.c
1 file changed, 27 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/52/152/1
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 1e10701..44b4508 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1339,6 +1339,23 @@
return rc;
}
+ /* For a few messages, mmctx may be NULL. For all others, we want to
+ * ensure a non-NULL mmctx. At the same time, we want to keep the
+ * message validity check intact, so that all message types appear in
+ * the switch statement and the default case thus means "unknown
+ * message". If we split the switch in two parts to check non-NULL
+ * halfway, the unknown-message check breaks, or we'd need to duplicate
+ * the switch cases in both parts. Instead, just have this macro to
+ * avoid too much code dup: */
+#define CHECK_MMCTX \
+ if (!mmctx) { \
+ LOGP(DMM, LOGL_ERROR, \
+ "Received GSM 04.08 message type 0x%02x," \
+ " but no MM context available\n", \
+ gh->msg_type); \
+ return -EINVAL; \
+ }
+
switch (gh->msg_type) {
case GSM48_MT_GMM_RA_UPD_REQ:
rc = gsm48_rx_gmm_ra_upd_req(mmctx, msg, llme);
@@ -1348,20 +1365,25 @@
break;
/* For all the following types mmctx can not be NULL */
case GSM48_MT_GMM_ID_RESP:
+ CHECK_MMCTX
rc = gsm48_rx_gmm_id_resp(mmctx, msg);
break;
case GSM48_MT_GMM_STATUS:
+ CHECK_MMCTX
rc = gsm48_rx_gmm_status(mmctx, msg);
break;
case GSM48_MT_GMM_DETACH_REQ:
+ CHECK_MMCTX
rc = gsm48_rx_gmm_det_req(mmctx, msg);
break;
case GSM48_MT_GMM_DETACH_ACK:
+ CHECK_MMCTX
LOGMMCTXP(LOGL_INFO, mmctx, "-> DETACH ACK\n");
mm_ctx_cleanup_free(mmctx, "GPRS DETACH ACK");
rc = 0;
break;
case GSM48_MT_GMM_ATTACH_COMPL:
+ CHECK_MMCTX
/* only in case SGSN offered new P-TMSI */
LOGMMCTXP(LOGL_INFO, mmctx, "-> ATTACH COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
@@ -1380,6 +1402,7 @@
osmo_signal_dispatch(SS_SGSN, S_SGSN_ATTACH, &sig_data);
break;
case GSM48_MT_GMM_RA_UPD_COMPL:
+ CHECK_MMCTX
/* only in case SGSN offered new P-TMSI */
LOGMMCTXP(LOGL_INFO, mmctx, "-> ROUTING AREA UPDATE COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
@@ -1398,6 +1421,7 @@
osmo_signal_dispatch(SS_SGSN, S_SGSN_UPDATE, &sig_data);
break;
case GSM48_MT_GMM_PTMSI_REALL_COMPL:
+ CHECK_MMCTX
LOGMMCTXP(LOGL_INFO, mmctx, "-> PTMSI REALLLICATION COMPLETE\n");
mmctx_timer_stop(mmctx, 3350);
mmctx->t3350_mode = GMM_T3350_MODE_NONE;
@@ -1409,6 +1433,7 @@
rc = 0;
break;
case GSM48_MT_GMM_AUTH_CIPH_RESP:
+ CHECK_MMCTX
rc = gsm48_rx_gmm_auth_ciph_resp(mmctx, msg);
break;
default:
@@ -1418,6 +1443,8 @@
break;
}
+#undef CHECK_MMCTX
+
return rc;
}
--
To view, visit https://gerrit.osmocom.org/152
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I10e6fee84abf05179f9e70981cdd295c57a58391
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr(a)sysmocom.de>
Review at https://gerrit.osmocom.org/154
gprs_gmm.c: Perform LLME operations only if we have one
In case the GMM message did not arrive over a Gb interface, there is no
LLME (and thus the associated pointer is NULL). Don't try to perform
operations on a NULL LLME.
Change-Id: I0299509d778915308e9ce46244d22283170ce18c
---
M openbsc/src/gprs/gprs_gmm.c
1 file changed, 7 insertions(+), 5 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/54/154/1
diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index fcaf6bd..e8c9bc6 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1208,10 +1208,12 @@
}
if (!mmctx) {
- /* send a XID reset to re-set all LLC sequence numbers
- * in the MS */
- LOGMMCTXP(LOGL_NOTICE, mmctx, "LLC XID RESET\n");
- gprs_llgmm_reset(llme);
+ if (llme) {
+ /* send a XID reset to re-set all LLC sequence numbers
+ * in the MS */
+ LOGMMCTXP(LOGL_NOTICE, mmctx, "LLC XID RESET\n");
+ gprs_llgmm_reset(llme);
+ }
/* The MS has to perform GPRS attach */
/* Device is still IMSI attached for CS but initiate GPRS ATTACH,
* see GSM 04.08, 4.7.5.1.4 and G.6 */
@@ -1314,7 +1316,7 @@
/* MMCTX can be NULL when called */
- if (!mmctx &&
+ if (llme && !mmctx &&
gh->msg_type != GSM48_MT_GMM_ATTACH_REQ &&
gh->msg_type != GSM48_MT_GMM_RA_UPD_REQ) {
LOGP(DMM, LOGL_NOTICE, "Cannot handle GMM for unknown MM CTX\n");
--
To view, visit https://gerrit.osmocom.org/154
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0299509d778915308e9ce46244d22283170ce18c
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge(a)gnumonks.org>