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;
-
fixeria