diff options
author | Pau Espin Pedrol <pespin@sysmocom.de> | 2021-06-30 16:03:06 +0200 |
---|---|---|
committer | Pau Espin Pedrol <pespin@sysmocom.de> | 2021-07-01 13:09:10 +0200 |
commit | 4f67a9bf4610c9e948f0d81f3b039f36675180a3 (patch) | |
tree | bf7f2aca513c0df2f2eaf2f4772a779e2558898b /src | |
parent | 1989a190664ca1d4169c80336b2ecc00426c84f1 (diff) |
pdch: Fix heap-use-after-free in pdch->ulc
In existing previous code, pdch->ulc would be freed in
gprs_rlcmac_pdch::free_resources() when it became disabled as per PCUIF
info_ind (for instance, when a DYN TS is switched PDCH->SDCCH8).
However, pdch->ulc was so far only allocated during pdch_init, which is
only called during bts_alloc() time.
Hence, after first info_ind disabling it, if it became again enabled
(again by info_ind re-enabling it after SDCCH8 was not longer in use),
the pdch->ulc would be used again but it would point to freed memory.
Let's rearrange how/when resources are freed to make it more logical.
With this patch, pdch internal resources are freed upon ->disable(), and
re-allocated upon ->enable().
Change-Id: Id51f5f6a54ac9f24b784c17bc360ac38f5726fc7
Diffstat (limited to 'src')
-rw-r--r-- | src/osmobts_sock.c | 3 | ||||
-rw-r--r-- | src/pcu_l1_if.cpp | 3 | ||||
-rw-r--r-- | src/pdch.cpp | 24 | ||||
-rw-r--r-- | src/pdch.h | 5 |
4 files changed, 22 insertions, 13 deletions
diff --git a/src/osmobts_sock.c b/src/osmobts_sock.c index 5c6415fb..91b62a00 100644 --- a/src/osmobts_sock.c +++ b/src/osmobts_sock.c @@ -121,7 +121,8 @@ static void pcu_sock_close(int lost) } #endif for (ts = 0; ts < 8; ts++) - pdch_disable(&bts->trx[trx].pdch[ts]); + if (pdch_is_enabled(&bts->trx[trx].pdch[ts])) + pdch_disable(&bts->trx[trx].pdch[ts]); /* FIXME: NOT ALL RESOURCES are freed in this case... inconsistent with the other code. Share the code with pcu_l1if.c for the reset. */ bts_trx_free_all_tbf(&bts->trx[trx]); diff --git a/src/pcu_l1_if.cpp b/src/pcu_l1_if.cpp index 2373f60c..5aa88494 100644 --- a/src/pcu_l1_if.cpp +++ b/src/pcu_l1_if.cpp @@ -650,7 +650,7 @@ bssgp_failed: for (trx_nr = 0; trx_nr < ARRAY_SIZE(bts->trx); trx_nr++) { bts->trx[trx_nr].arfcn = info_ind->trx[trx_nr].arfcn; for (ts_nr = 0; ts_nr < ARRAY_SIZE(bts->trx[0].pdch); ts_nr++) - bts->trx[trx_nr].pdch[ts_nr].free_resources(); + bts->trx[trx_nr].pdch[ts_nr].disable(); } gprs_bssgp_destroy(bts); exit(0); @@ -818,7 +818,6 @@ bssgp_failed: } else { if (pdch->is_enabled()) { pcu_tx_act_req(bts, pdch, 0); - pdch->free_resources(); pdch->disable(); } } diff --git a/src/pdch.cpp b/src/pdch.cpp index 9321384b..2ec40ce1 100644 --- a/src/pdch.cpp +++ b/src/pdch.cpp @@ -139,19 +139,24 @@ void pdch_init(struct gprs_rlcmac_pdch *pdch, struct gprs_rlcmac_trx *trx, uint8 /* Initialize the PTCCH/D message (Packet Timing Advance Control Channel) */ memset(pdch->ptcch_msg, PTCCH_TAI_FREE, PTCCH_TAI_NUM); memset(pdch->ptcch_msg + PTCCH_TAI_NUM, PTCCH_PADDING, 7); - pdch->ulc = pdch_ulc_alloc(pdch, trx->bts); } void gprs_rlcmac_pdch::enable() { - /* TODO: Check if there are still allocated resources.. */ + OSMO_ASSERT(m_is_enabled == 0); INIT_LLIST_HEAD(&paging_list); + + OSMO_ASSERT(!this->ulc); + this->ulc = pdch_ulc_alloc(this, trx->bts); + m_is_enabled = 1; } void gprs_rlcmac_pdch::disable() { - /* TODO.. kick free_resources once we know the TRX/TS we are on */ + OSMO_ASSERT(m_is_enabled == 1); + this->free_resources(); + m_is_enabled = 0; } @@ -159,10 +164,6 @@ void gprs_rlcmac_pdch::free_resources() { struct gprs_rlcmac_paging *pag; - /* we are not enabled. there should be no resources */ - if (!is_enabled()) - return; - /* kick all TBF on slot */ pdch_free_all_tbf(this); @@ -171,6 +172,7 @@ void gprs_rlcmac_pdch::free_resources() talloc_free(pag); talloc_free(this->ulc); + this->ulc = NULL; } struct gprs_rlcmac_paging *gprs_rlcmac_pdch::dequeue_paging() @@ -1174,6 +1176,12 @@ void pdch_free_all_tbf(struct gprs_rlcmac_pdch *pdch) } } -void pdch_disable(struct gprs_rlcmac_pdch *pdch) { +void pdch_disable(struct gprs_rlcmac_pdch *pdch) +{ pdch->disable(); } + +bool pdch_is_enabled(const struct gprs_rlcmac_pdch *pdch) +{ + return pdch->is_enabled(); +} @@ -55,8 +55,6 @@ struct gprs_rlcmac_pdch { bool add_paging(uint8_t chan_needed, const struct osmo_mobile_identity *mi); - void free_resources(); - bool is_enabled() const; void enable(); @@ -145,6 +143,8 @@ private: enum gprs_rlcmac_tbf_direction dir); gprs_rlcmac_tbf *tbf_by_tfi(uint8_t tfi, enum gprs_rlcmac_tbf_direction dir); + + void free_resources(); #endif uint8_t m_num_tbfs[2]; @@ -191,6 +191,7 @@ extern "C" { void pdch_init(struct gprs_rlcmac_pdch *pdch, struct gprs_rlcmac_trx *trx, uint8_t ts_nr); void pdch_free_all_tbf(struct gprs_rlcmac_pdch *pdch); void pdch_disable(struct gprs_rlcmac_pdch *pdch); +bool pdch_is_enabled(const struct gprs_rlcmac_pdch *pdch); #ifdef __cplusplus } #endif |