Change in osmocom-bb[master]: layer23/l1ctl.c: clean up & fix message length checking

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/gerrit-log@lists.osmocom.org/.

Vadim Yanitskiy gerrit-no-reply at lists.osmocom.org
Wed Oct 3 12:47:25 UTC 2018


Vadim Yanitskiy has uploaded this change for review. ( https://gerrit.osmocom.org/11219


Change subject: layer23/l1ctl.c: clean up & fix message length checking
......................................................................

layer23/l1ctl.c: clean up & fix message length checking

Almost all handlers for received L1CTL messages are also affected
by the bug fixed in I7fe2e00bb45ba07c9bb7438445eededfa09c96f3. In
short, they do verify the length of 'msg->l2h' or 'msg->l3h', but
not the 'msg->l1h'. Let's fix this, and also add missing checks.

Change-Id: I866bb5d97a1cc1b6cb887877bb444b9e3dca977a
---
M src/host/layer23/src/common/l1ctl.c
1 file changed, 44 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/19/11219/1

diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c
index 642cde8..6f4a6d8 100644
--- a/src/host/layer23/src/common/l1ctl.c
+++ b/src/host/layer23/src/common/l1ctl.c
@@ -83,9 +83,9 @@
 	struct gsm_time tm;
 	struct osmobb_fbsb_res fr;
 
-	if (msgb_l3len(msg) < sizeof(*dl) + sizeof(*sb)) {
-		LOGP(DL1C, LOGL_ERROR, "FBSB RESP: MSG too short %u\n",
-			msgb_l3len(msg));
+	if (msgb_l1len(msg) < (sizeof(*dl) + sizeof(*sb))) {
+		LOGP(DL1C, LOGL_ERROR, "FBSB RESP: MSG too short (len=%u), "
+			"missing UL info header and/or payload\n", msgb_l1len(msg));
 		return -1;
 	}
 
@@ -121,9 +121,9 @@
 	struct osmo_phsap_prim pp;
 	struct l1ctl_info_dl *dl;
 
-	if (msgb_l2len(msg) < sizeof(*dl)) {
-		LOGP(DL1C, LOGL_ERROR, "RACH CONF: MSG too short %u\n",
-			msgb_l3len(msg));
+	if (msgb_l1len(msg) < sizeof(*dl)) {
+		LOGP(DL1C, LOGL_ERROR, "RACH CONF MSG too short "
+			"(len=%u), missing DL info header\n", msgb_l1len(msg));
 		msgb_free(msg);
 		return -1;
 	}
@@ -150,9 +150,9 @@
 	uint8_t gsmtap_chan_type;
 	struct gsm_time tm;
 
-	if (msgb_l3len(msg) < sizeof(*ccch)) {
-		LOGP(DL1C, LOGL_ERROR, "MSG too short Data Ind: %u\n",
-			msgb_l3len(msg));
+	if (msgb_l1len(msg) < sizeof(*dl)) {
+		LOGP(DL1C, LOGL_ERROR, "DATA IND MSG too short (len=%u), "
+			"missing UL info header\n", msgb_l1len(msg));
 		msgb_free(msg);
 		return -1;
 	}
@@ -260,6 +260,13 @@
 	struct l1ctl_info_dl *dl = (struct l1ctl_info_dl *) msg->l1h;
 	struct lapdm_entity *le;
 
+	if (msgb_l1len(msg) < sizeof(*dl)) {
+		LOGP(DL1C, LOGL_ERROR, "DATA CONF MSG too short (len=%u), "
+			"missing UL info header\n", msgb_l1len(msg));
+		msgb_free(msg);
+		return -1;
+	}
+
 	osmo_prim_init(&pp.oph, SAP_GSM_PH, PRIM_PH_RTS,
 			PRIM_OP_INDICATION, msg);
 
@@ -284,14 +291,12 @@
 
 	DEBUGP(DL1C, "(%s)\n", osmo_hexdump(msg->l2h, msgb_l2len(msg)));
 
-	if (msgb_l2len(msg) > 23) {
-		LOGP(DL1C, LOGL_ERROR, "L1 cannot handle message length "
-			"> 23 (%u)\n", msgb_l2len(msg));
+	if (msgb_l2len(msg) != 23) {
+		LOGP(DL1C, LOGL_ERROR, "Wrong message length (len=%u), "
+			"DATA REQ ignored, please fix!\n", msgb_l2len(msg));
 		msgb_free(msg);
 		return -EINVAL;
-	} else if (msgb_l2len(msg) < 23)
-		LOGP(DL1C, LOGL_ERROR, "L1 message length < 23 (%u) "
-			"doesn't seem right!\n", msgb_l2len(msg));
+	}
 
 	/* send copy via GSMTAP */
 	rsl_dec_chan_nr(chan_nr, &chan_type, &chan_ss, &chan_ts);
@@ -702,6 +707,12 @@
 {
 	struct l1ctl_pm_conf *pmr;
 
+	if (msgb_l1len(msg) < sizeof(*pmr)) {
+		LOGP(DL1C, LOGL_ERROR, "PM CONF MSG too short (len=%u), "
+			"missing measurement results\n", msgb_l1len(msg));
+		return -1;
+	}
+
 	for (pmr = (struct l1ctl_pm_conf *) msg->l1h;
 	     (uint8_t *) pmr < msg->tail; pmr++) {
 		struct osmobb_meas_res mr;
@@ -721,9 +732,9 @@
 	struct osmobb_ccch_mode_conf mc;
 	struct l1ctl_ccch_mode_conf *conf;
 
-	if (msgb_l3len(msg) < sizeof(*conf)) {
-		LOGP(DL1C, LOGL_ERROR, "CCCH MODE CONF: MSG too short %u\n",
-			msgb_l3len(msg));
+	if (msgb_l1len(msg) < sizeof(*conf)) {
+		LOGP(DL1C, LOGL_ERROR, "CCCH MODE CONF: MSG too short "
+			"(len=%u), missing CCCH mode info\n", msgb_l1len(msg));
 		return -1;
 	}
 
@@ -744,9 +755,9 @@
 	struct osmobb_tch_mode_conf mc;
 	struct l1ctl_tch_mode_conf *conf;
 
-	if (msgb_l3len(msg) < sizeof(*conf)) {
-		LOGP(DL1C, LOGL_ERROR, "TCH MODE CONF: MSG too short %u\n",
-			msgb_l3len(msg));
+	if (msgb_l1len(msg) < sizeof(*conf)) {
+		LOGP(DL1C, LOGL_ERROR, "TCH MODE CONF: MSG too short "
+			"(len=%u), missing TCH mode info\n", msgb_l1len(msg));
 		return -1;
 	}
 
@@ -768,6 +779,12 @@
 	struct l1ctl_info_dl *dl;
 	struct l1ctl_traffic_ind *ti;
 
+	if (msgb_l1len(msg) < sizeof(*dl)) {
+		LOGP(DL1C, LOGL_ERROR, "TRAFFIC IND MSG too short "
+			"(len=%u), missing DL info header\n", msgb_l1len(msg));
+		return -1;
+	}
+
 	/* Header handling */
 	dl = (struct l1ctl_info_dl *) msg->l1h;
 	msg->l2h = dl->payload;
@@ -854,6 +871,12 @@
 {
 	struct l1ctl_neigh_pm_ind *pm_ind;
 
+	if (msgb_l1len(msg) < sizeof(*pm_ind)) {
+		LOGP(DL1C, LOGL_ERROR, "NEIGH PH IND MSG too short "
+			"(len=%u), missing measurement results\n", msgb_l1len(msg));
+		return -1;
+	}
+
 	for (pm_ind = (struct l1ctl_neigh_pm_ind *) msg->l1h;
 	     (uint8_t *) pm_ind < msg->tail; pm_ind++) {
 		struct osmobb_neigh_pm_ind mi;

-- 
To view, visit https://gerrit.osmocom.org/11219
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I866bb5d97a1cc1b6cb887877bb444b9e3dca977a
Gerrit-Change-Number: 11219
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181003/2cfa190f/attachment.htm>


More information about the gerrit-log mailing list