[PATCH] openbsc[master]: gprs_gmm.c: Don't try to de-reference NULL mmctx

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Jun 2 01:02:16 UTC 2016


Review at  https://gerrit.osmocom.org/171

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: I7908de65bec91599f7042549b832cbbd7ae5a9a8
---
M openbsc/src/gprs/gprs_gmm.c
1 file changed, 32 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/71/171/1

diff --git a/openbsc/src/gprs/gprs_gmm.c b/openbsc/src/gprs/gprs_gmm.c
index 1e10701..e788e3d 100644
--- a/openbsc/src/gprs/gprs_gmm.c
+++ b/openbsc/src/gprs/gprs_gmm.c
@@ -1339,6 +1339,15 @@
 		return rc;
 	}
 
+	/*
+	 * For a few messages, mmctx may be NULL. For most, 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. Just keep one large switch and add some gotos.
+	 */
 	switch (gh->msg_type) {
 	case GSM48_MT_GMM_RA_UPD_REQ:
 		rc = gsm48_rx_gmm_ra_upd_req(mmctx, msg, llme);
@@ -1348,20 +1357,30 @@
 		break;
 	/* For all the following types mmctx can not be NULL */
 	case GSM48_MT_GMM_ID_RESP:
+		if (!mmctx)
+			goto null_mmctx;
 		rc = gsm48_rx_gmm_id_resp(mmctx, msg);
 		break;
 	case GSM48_MT_GMM_STATUS:
+		if (!mmctx)
+			goto null_mmctx;
 		rc = gsm48_rx_gmm_status(mmctx, msg);
 		break;
 	case GSM48_MT_GMM_DETACH_REQ:
+		if (!mmctx)
+			goto null_mmctx;
 		rc = gsm48_rx_gmm_det_req(mmctx, msg);
 		break;
 	case GSM48_MT_GMM_DETACH_ACK:
+		if (!mmctx)
+			goto null_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:
+		if (!mmctx)
+			goto null_mmctx;
 		/* only in case SGSN offered new P-TMSI */
 		LOGMMCTXP(LOGL_INFO, mmctx, "-> ATTACH COMPLETE\n");
 		mmctx_timer_stop(mmctx, 3350);
@@ -1380,6 +1399,8 @@
 		osmo_signal_dispatch(SS_SGSN, S_SGSN_ATTACH, &sig_data);
 		break;
 	case GSM48_MT_GMM_RA_UPD_COMPL:
+		if (!mmctx)
+			goto null_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 +1419,8 @@
 		osmo_signal_dispatch(SS_SGSN, S_SGSN_UPDATE, &sig_data);
 		break;
 	case GSM48_MT_GMM_PTMSI_REALL_COMPL:
+		if (!mmctx)
+			goto null_mmctx;
 		LOGMMCTXP(LOGL_INFO, mmctx, "-> PTMSI REALLLICATION COMPLETE\n");
 		mmctx_timer_stop(mmctx, 3350);
 		mmctx->t3350_mode = GMM_T3350_MODE_NONE;
@@ -1409,6 +1432,8 @@
 		rc = 0;
 		break;
 	case GSM48_MT_GMM_AUTH_CIPH_RESP:
+		if (!mmctx)
+			goto null_mmctx;
 		rc = gsm48_rx_gmm_auth_ciph_resp(mmctx, msg);
 		break;
 	default:
@@ -1419,6 +1444,13 @@
 	}
 
 	return rc;
+
+null_mmctx:
+	LOGP(DMM, LOGL_ERROR,
+	     "Received GSM 04.08 message type 0x%02x,"
+	     " but no MM context available\n",
+	     gh->msg_type);
+	return -EINVAL;
 }
 
 static void mmctx_timer_cb(void *_mm)

-- 
To view, visit https://gerrit.osmocom.org/171
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7908de65bec91599f7042549b832cbbd7ae5a9a8
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the OpenBSC mailing list