<p>Vadim Yanitskiy has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/11219">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">layer23/l1ctl.c: clean up & fix message length checking<br><br>Almost all handlers for received L1CTL messages are also affected<br>by the bug fixed in I7fe2e00bb45ba07c9bb7438445eededfa09c96f3. In<br>short, they do verify the length of 'msg->l2h' or 'msg->l3h', but<br>not the 'msg->l1h'. Let's fix this, and also add missing checks.<br><br>Change-Id: I866bb5d97a1cc1b6cb887877bb444b9e3dca977a<br>---<br>M src/host/layer23/src/common/l1ctl.c<br>1 file changed, 44 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/19/11219/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/host/layer23/src/common/l1ctl.c b/src/host/layer23/src/common/l1ctl.c</span><br><span>index 642cde8..6f4a6d8 100644</span><br><span>--- a/src/host/layer23/src/common/l1ctl.c</span><br><span>+++ b/src/host/layer23/src/common/l1ctl.c</span><br><span>@@ -83,9 +83,9 @@</span><br><span> struct gsm_time tm;</span><br><span> struct osmobb_fbsb_res fr;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l3len(msg) < sizeof(*dl) + sizeof(*sb)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "FBSB RESP: MSG too short %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l3len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < (sizeof(*dl) + sizeof(*sb))) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "FBSB RESP: MSG too short (len=%u), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "missing UL info header and/or payload\n", msgb_l1len(msg));</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span>@@ -121,9 +121,9 @@</span><br><span> struct osmo_phsap_prim pp;</span><br><span> struct l1ctl_info_dl *dl;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l2len(msg) < sizeof(*dl)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "RACH CONF: MSG too short %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l3len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*dl)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "RACH CONF MSG too short "</span><br><span style="color: hsl(120, 100%, 40%);">+ "(len=%u), missing DL info header\n", msgb_l1len(msg));</span><br><span> msgb_free(msg);</span><br><span> return -1;</span><br><span> }</span><br><span>@@ -150,9 +150,9 @@</span><br><span> uint8_t gsmtap_chan_type;</span><br><span> struct gsm_time tm;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l3len(msg) < sizeof(*ccch)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "MSG too short Data Ind: %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l3len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*dl)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "DATA IND MSG too short (len=%u), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "missing UL info header\n", msgb_l1len(msg));</span><br><span> msgb_free(msg);</span><br><span> return -1;</span><br><span> }</span><br><span>@@ -260,6 +260,13 @@</span><br><span> struct l1ctl_info_dl *dl = (struct l1ctl_info_dl *) msg->l1h;</span><br><span> struct lapdm_entity *le;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*dl)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "DATA CONF MSG too short (len=%u), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "missing UL info header\n", msgb_l1len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_free(msg);</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> osmo_prim_init(&pp.oph, SAP_GSM_PH, PRIM_PH_RTS,</span><br><span> PRIM_OP_INDICATION, msg);</span><br><span> </span><br><span>@@ -284,14 +291,12 @@</span><br><span> </span><br><span> DEBUGP(DL1C, "(%s)\n", osmo_hexdump(msg->l2h, msgb_l2len(msg)));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l2len(msg) > 23) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "L1 cannot handle message length "</span><br><span style="color: hsl(0, 100%, 40%);">- "> 23 (%u)\n", msgb_l2len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l2len(msg) != 23) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "Wrong message length (len=%u), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "DATA REQ ignored, please fix!\n", msgb_l2len(msg));</span><br><span> msgb_free(msg);</span><br><span> return -EINVAL;</span><br><span style="color: hsl(0, 100%, 40%);">- } else if (msgb_l2len(msg) < 23)</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "L1 message length < 23 (%u) "</span><br><span style="color: hsl(0, 100%, 40%);">- "doesn't seem right!\n", msgb_l2len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> /* send copy via GSMTAP */</span><br><span> rsl_dec_chan_nr(chan_nr, &chan_type, &chan_ss, &chan_ts);</span><br><span>@@ -702,6 +707,12 @@</span><br><span> {</span><br><span> struct l1ctl_pm_conf *pmr;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*pmr)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "PM CONF MSG too short (len=%u), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "missing measurement results\n", msgb_l1len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> for (pmr = (struct l1ctl_pm_conf *) msg->l1h;</span><br><span> (uint8_t *) pmr < msg->tail; pmr++) {</span><br><span> struct osmobb_meas_res mr;</span><br><span>@@ -721,9 +732,9 @@</span><br><span> struct osmobb_ccch_mode_conf mc;</span><br><span> struct l1ctl_ccch_mode_conf *conf;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l3len(msg) < sizeof(*conf)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "CCCH MODE CONF: MSG too short %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l3len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*conf)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "CCCH MODE CONF: MSG too short "</span><br><span style="color: hsl(120, 100%, 40%);">+ "(len=%u), missing CCCH mode info\n", msgb_l1len(msg));</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span>@@ -744,9 +755,9 @@</span><br><span> struct osmobb_tch_mode_conf mc;</span><br><span> struct l1ctl_tch_mode_conf *conf;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (msgb_l3len(msg) < sizeof(*conf)) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DL1C, LOGL_ERROR, "TCH MODE CONF: MSG too short %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_l3len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*conf)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "TCH MODE CONF: MSG too short "</span><br><span style="color: hsl(120, 100%, 40%);">+ "(len=%u), missing TCH mode info\n", msgb_l1len(msg));</span><br><span> return -1;</span><br><span> }</span><br><span> </span><br><span>@@ -768,6 +779,12 @@</span><br><span> struct l1ctl_info_dl *dl;</span><br><span> struct l1ctl_traffic_ind *ti;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*dl)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "TRAFFIC IND MSG too short "</span><br><span style="color: hsl(120, 100%, 40%);">+ "(len=%u), missing DL info header\n", msgb_l1len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* Header handling */</span><br><span> dl = (struct l1ctl_info_dl *) msg->l1h;</span><br><span> msg->l2h = dl->payload;</span><br><span>@@ -854,6 +871,12 @@</span><br><span> {</span><br><span> struct l1ctl_neigh_pm_ind *pm_ind;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (msgb_l1len(msg) < sizeof(*pm_ind)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DL1C, LOGL_ERROR, "NEIGH PH IND MSG too short "</span><br><span style="color: hsl(120, 100%, 40%);">+ "(len=%u), missing measurement results\n", msgb_l1len(msg));</span><br><span style="color: hsl(120, 100%, 40%);">+ return -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> for (pm_ind = (struct l1ctl_neigh_pm_ind *) msg->l1h;</span><br><span> (uint8_t *) pm_ind < msg->tail; pm_ind++) {</span><br><span> struct osmobb_neigh_pm_ind mi;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11219">change 11219</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11219"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmocom-bb </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I866bb5d97a1cc1b6cb887877bb444b9e3dca977a </div>
<div style="display:none"> Gerrit-Change-Number: 11219 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Vadim Yanitskiy <axilirator@gmail.com> </div>