fixeria has submitted this change. (
https://gerrit.osmocom.org/c/osmocom-bb/+/30329 )
Change subject: layer23: always check return value of rsl_dec_chan_nr()
......................................................................
layer23: always check return value of rsl_dec_chan_nr()
The rsl_dec_chan_nr() may fail to decode RSL channel number, so
variables ch_type/chan_ss/chan_ts would remain uninitialized.
Change-Id: I9ab18bdaf41a29fcd32a7060668ef9db07b8cf7e
Related: OS#5599
---
M src/host/layer23/src/common/l1ctl.c
M src/host/layer23/src/misc/app_cbch_sniff.c
M src/host/layer23/src/misc/app_ccch_scan.c
M src/host/layer23/src/misc/cell_log.c
M src/host/layer23/src/mobile/gsm48_rr.c
5 files changed, 146 insertions(+), 31 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
diff --git a/src/host/layer23/src/common/l1ctl.c
b/src/host/layer23/src/common/l1ctl.c
index ae7be6e..7482788 100644
--- a/src/host/layer23/src/common/l1ctl.c
+++ b/src/host/layer23/src/common/l1ctl.c
@@ -229,7 +229,13 @@
ccch = (struct l1ctl_data_ind *) msg->l2h;
gsm_fn2gsmtime(&tm, ntohl(dl->frame_nr));
- rsl_dec_chan_nr(dl->chan_nr, &chan_type, &chan_ss, &chan_ts);
+ if (rsl_dec_chan_nr(dl->chan_nr, &chan_type, &chan_ss, &chan_ts) != 0) {
+ LOGP(DL1C, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, dl->chan_nr);
+ return -EINVAL;
+ }
+
DEBUGP(DL1C, "%s (%.4u/%.2u/%.2u) %d dBm: %s\n",
rsl_chan_nr_str(dl->chan_nr), tm.t1, tm.t2, tm.t3,
(int)dl->rx_level-110,
@@ -374,7 +380,6 @@
struct l1ctl_hdr *l1h;
struct l1ctl_info_ul *l1i_ul;
uint8_t chan_type, chan_ts, chan_ss;
- uint8_t gsmtap_chan_type;
DEBUGP(DL1C, "(%s)\n", osmo_hexdump(msg->l2h, msgb_l2len(msg)));
@@ -386,11 +391,16 @@
}
/* send copy via GSMTAP */
- rsl_dec_chan_nr(chan_nr, &chan_type, &chan_ss, &chan_ts);
- gsmtap_chan_type = chantype_rsl2gsmtap2(chan_type, link_id, false);
- gsmtap_send(gsmtap_inst, ms->rrlayer.cd_now.arfcn |
GSMTAP_ARFCN_F_UPLINK,
- chan_ts, gsmtap_chan_type, chan_ss, 0, 127, 255,
- msg->l2h, msgb_l2len(msg));
+ if (rsl_dec_chan_nr(chan_nr, &chan_type, &chan_ss, &chan_ts) == 0) {
+ uint8_t gsmtap_chan_type = chantype_rsl2gsmtap2(chan_type,
link_id, false);
+ gsmtap_send(gsmtap_inst, ms->rrlayer.cd_now.arfcn |
GSMTAP_ARFCN_F_UPLINK,
+ chan_ts, gsmtap_chan_type, chan_ss, 0, 127, 255,
+ msg->l2h, msgb_l2len(msg));
+ } else {
+ LOGP(DL1C, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, chan_nr);
+ }
/* prepend uplink info header */
l1i_ul = (struct l1ctl_info_ul *) msgb_push(msg, sizeof(*l1i_ul));
diff --git a/src/host/layer23/src/misc/app_cbch_sniff.c
b/src/host/layer23/src/misc/app_cbch_sniff.c
index 33b1dfa..3ef1449 100644
--- a/src/host/layer23/src/misc/app_cbch_sniff.c
+++ b/src/host/layer23/src/misc/app_cbch_sniff.c
@@ -118,7 +118,13 @@
}
msg->l3h = (uint8_t *) TLVP_VAL(&tv, RSL_IE_L3_INFO);
- rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rllh->chan_nr);
+ return -EINVAL;
+ }
+
switch (ch_type) {
case RSL_CHAN_BCCH:
return bcch(ms, msg);
diff --git a/src/host/layer23/src/misc/app_ccch_scan.c
b/src/host/layer23/src/misc/app_ccch_scan.c
index dc16359..04afdcf 100644
--- a/src/host/layer23/src/misc/app_ccch_scan.c
+++ b/src/host/layer23/src/misc/app_ccch_scan.c
@@ -166,7 +166,12 @@
if (ia->page_mode & 0xf0)
return 0;
- rsl_dec_chan_nr(ia->chan_desc.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(ia->chan_desc.chan_nr, &ch_type, &ch_subch, &ch_ts)
!= 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, ia->chan_desc.chan_nr);
+ return -EINVAL;
+ }
if (!ia->chan_desc.h0.h) {
/* Non-hopping */
diff --git a/src/host/layer23/src/misc/cell_log.c
b/src/host/layer23/src/misc/cell_log.c
index cf56a8d..3cd34b8 100644
--- a/src/host/layer23/src/misc/cell_log.c
+++ b/src/host/layer23/src/misc/cell_log.c
@@ -680,7 +680,13 @@
return -EINVAL;
}
- rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rllh->chan_nr);
+ return -EINVAL;
+ }
+
switch (ch_type) {
case RSL_CHAN_PCH_AGCH:
return pch_agch(ms, msg);
diff --git a/src/host/layer23/src/mobile/gsm48_rr.c
b/src/host/layer23/src/mobile/gsm48_rr.c
index fa11001..884e426 100644
--- a/src/host/layer23/src/mobile/gsm48_rr.c
+++ b/src/host/layer23/src/mobile/gsm48_rr.c
@@ -247,8 +247,14 @@
struct gsm_settings *set = &ms->settings;
uint8_t ch_type, ch_subch, ch_ts;
+ if (rsl_dec_chan_nr(chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, chan_nr);
+ return GSM48_RR_CAUSE_CHAN_MODE_UNACCT;
+ }
+
/* only complain if we use TCH/F or TCH/H */
- rsl_dec_chan_nr(chan_nr, &ch_type, &ch_subch, &ch_ts);
if (ch_type != RSL_CHAN_Bm_ACCHs
&& ch_type != RSL_CHAN_Lm_ACCHs)
return 0;
@@ -648,15 +654,15 @@
gsm_print_mcc(cs->sel_mcc),
gsm_print_mnc(cs->sel_mnc), cs->sel_lac, cs->sel_id);
if (rr->state == GSM48_RR_ST_DEDICATED) {
- rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type,
- &ch_subch, &ch_ts);
sprintf(text + strlen(text), " TA=%d pwr=%d TS=%d",
rr->cd_now.ind_ta - set->alter_delay,
(set->alter_tx_power) ? set->alter_tx_power_value
: rr->cd_now.ind_tx_power, ch_ts);
- if (ch_type == RSL_CHAN_SDCCH8_ACCH
- || ch_type == RSL_CHAN_SDCCH4_ACCH)
- sprintf(text + strlen(text), "/%d", ch_subch);
+ if (rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type,
&ch_subch, &ch_ts) == 0) {
+ if (ch_type == RSL_CHAN_SDCCH8_ACCH
+ || ch_type == RSL_CHAN_SDCCH4_ACCH) // TODO:
TCH/H
+ sprintf(text + strlen(text), "/%d",
ch_subch);
+ }
} else
gsm322_meas(rr->ms, rxlev);
}
@@ -2428,7 +2434,12 @@
/* decode channel description */
LOGP(DRR, LOGL_INFO, "IMMEDIATE ASSIGNMENT:\n");
cd.chan_nr = ia->chan_desc.chan_nr;
- rsl_dec_chan_nr(cd.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cd.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd.chan_nr);
+ return -EINVAL;
+ }
if (ia->chan_desc.h0.h) {
cd.h = 1;
gsm48_decode_chan_h1(&ia->chan_desc, &cd.tsc, &cd.maio,
@@ -2542,7 +2553,12 @@
/* decode channel description */
LOGP(DRR, LOGL_INFO, "IMMEDIATE ASSIGNMENT EXTENDED:\n");
cd1.chan_nr = ia->chan_desc1.chan_nr;
- rsl_dec_chan_nr(cd1.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cd1.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd1.chan_nr);
+ return -EINVAL;
+ }
if (ia->chan_desc1.h0.h) {
cd1.h = 1;
gsm48_decode_chan_h1(&ia->chan_desc1, &cd1.tsc, &cd1.maio,
@@ -2566,7 +2582,12 @@
gsm_print_arfcn(cd1.arfcn), ch_ts, ch_subch, cd1.tsc);
}
cd2.chan_nr = ia->chan_desc2.chan_nr;
- rsl_dec_chan_nr(cd2.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cd2.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd2.chan_nr);
+ return -EINVAL;
+ }
if (ia->chan_desc2.h0.h) {
cd2.h = 1;
gsm48_decode_chan_h1(&ia->chan_desc2, &cd2.tsc, &cd2.maio,
@@ -2986,7 +3007,14 @@
/* establish */
LOGP(DRR, LOGL_INFO, "establishing channel in dedicated mode\n");
- rsl_dec_chan_nr(cd->chan_nr, &ch_type, &ch_subch, &ch_ts);
+
+ if (rsl_dec_chan_nr(cd->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd->chan_nr);
+ return -EINVAL;
+ }
+
LOGP(DRR, LOGL_INFO, " Channel type %d, subch %d, ts %d, mode %d, "
"audio-mode %d, cipher %d\n", ch_type, ch_subch, ch_ts,
cd->mode, rr->audio_mode, rr->cipher_type + 1);
@@ -3428,8 +3456,14 @@
struct gsm48_rrlayer *rr = &ms->rrlayer;
uint8_t ch_type, ch_subch, ch_ts;
+ if (rsl_dec_chan_nr(chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, chan_nr);
+ return -EINVAL;
+ }
+
/* only apply mode to TCH/F or TCH/H */
- rsl_dec_chan_nr(chan_nr, &ch_type, &ch_subch, &ch_ts);
if (ch_type != RSL_CHAN_Bm_ACCHs
&& ch_type != RSL_CHAN_Lm_ACCHs)
return -ENOTSUP;
@@ -3474,7 +3508,12 @@
/* decode channel description */
LOGP(DRR, LOGL_INFO, "FREQUENCY REDEFINITION:\n");
cd.chan_nr = fr->chan_desc.chan_nr;
- rsl_dec_chan_nr(cd.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cd.chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd.chan_nr);
+ return -EINVAL;
+ }
if (fr->chan_desc.h0.h) {
cd.h = 1;
gsm48_decode_chan_h1(&fr->chan_desc, &cd.tsc, &cd.maio,
@@ -3578,7 +3617,12 @@
/* decode channel description */
cd->chan_nr = cm->chan_desc.chan_nr;
- rsl_dec_chan_nr(cd->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cd->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cd->chan_nr);
+ return -EINVAL;
+ }
if (cm->chan_desc.h0.h) {
cd->h = 1;
gsm48_decode_chan_h1(&cm->chan_desc, &cd->tsc, &cd->maio,
@@ -3701,7 +3745,12 @@
struct gsm48_chan_desc *ccd = (struct gsm48_chan_desc *)
TLVP_VAL(&tp, GSM48_IE_CH_DESC_1_BEFORE);
cdb->chan_nr = ccd->chan_nr;
- rsl_dec_chan_nr(cdb->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cdb->chan_nr, &ch_type, &ch_subch, &ch_ts)
!= 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cdb->chan_nr);
+ return -EINVAL;
+ }
if (ccd->h0.h) {
cdb->h = 1;
gsm48_decode_chan_h1(ccd, &cdb->tsc, &cdb->maio,
@@ -3724,7 +3773,12 @@
/* decode channel description (after time) */
cda->chan_nr = ac->chan_desc.chan_nr;
- rsl_dec_chan_nr(cda->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cda->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cda->chan_nr);
+ return -EINVAL;
+ }
if (ac->chan_desc.h0.h) {
cda->h = 1;
gsm48_decode_chan_h1(&ac->chan_desc, &cda->tsc, &cda->maio,
@@ -4090,7 +4144,12 @@
struct gsm48_chan_desc *ccd = (struct gsm48_chan_desc *)
TLVP_VAL(&tp, GSM48_IE_CH_DESC_1_BEFORE);
cdb->chan_nr = ccd->chan_nr;
- rsl_dec_chan_nr(cdb->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cdb->chan_nr, &ch_type, &ch_subch, &ch_ts)
!= 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cdb->chan_nr);
+ return -EINVAL;
+ }
if (ccd->h0.h) {
cdb->h = 1;
gsm48_decode_chan_h1(ccd, &cdb->tsc, &cdb->maio,
@@ -4113,7 +4172,12 @@
/* decode channel description (after time) */
cda->chan_nr = ho->chan_desc.chan_nr;
- rsl_dec_chan_nr(cda->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(cda->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRR, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, cda->chan_nr);
+ return -EINVAL;
+ }
if (ho->chan_desc.h0.h) {
cda->h = 1;
gsm48_decode_chan_h1(&ho->chan_desc, &cda->tsc, &cda->maio,
@@ -4843,7 +4907,13 @@
// TODO: timer depends on BCCH config
}
- rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rllh->chan_nr, &ch_type, &ch_subch, &ch_ts) != 0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rllh->chan_nr);
+ return -EINVAL;
+ }
+
switch (ch_type) {
case RSL_CHAN_PCH_AGCH:
return gsm48_rr_rx_pch_agch(ms, msg);
@@ -5167,7 +5237,13 @@
return gsm48_rr_upmsg(ms, nmsg);
}
- rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts) !=
0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rr->cd_now.chan_nr);
+ return -EINVAL;
+ }
+
if (ch_type != RSL_CHAN_Bm_ACCHs
&& ch_type != RSL_CHAN_Lm_ACCHs) {
LOGP(DRR, LOGL_INFO, "Requesting DCCH link, because no TCH "
@@ -5683,7 +5759,13 @@
goto error;
}
- rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts) !=
0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rr->cd_now.chan_nr);
+ goto error;
+ }
+
if (ch_type != RSL_CHAN_Bm_ACCHs) {
LOGP(DRR, LOGL_INFO, "Current channel is not (yet) TCH/F\n");
goto error;
@@ -5711,7 +5793,13 @@
if (!rr->dm_est)
return 0;
- rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts);
+ if (rsl_dec_chan_nr(rr->cd_now.chan_nr, &ch_type, &ch_subch, &ch_ts) !=
0) {
+ LOGP(DRSL, LOGL_ERROR,
+ "%s(): rsl_dec_chan_nr(chan_nr=0x%02x) failed\n",
+ __func__, rr->cd_now.chan_nr);
+ return -EINVAL;
+ }
+
if (ch_type != RSL_CHAN_Bm_ACCHs
&& ch_type != RSL_CHAN_Lm_ACCHs)
return 0;
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30329
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: I9ab18bdaf41a29fcd32a7060668ef9db07b8cf7e
Gerrit-Change-Number: 30329
Gerrit-PatchSet: 3
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-CC: msuraev <[email protected]>
Gerrit-MessageType: merged
