diff options
author | Harald Welte <laforge@gnumonks.org> | 2016-01-04 20:06:12 +0100 |
---|---|---|
committer | Harald Welte <laforge@gnumonks.org> | 2016-01-04 20:17:32 +0100 |
commit | 21b020b33633683d7c785af15c773aab0f79d0de (patch) | |
tree | d42861b6f96228d9955c9938658e03a19388a75e /src/osmo-bts-sysmo/l1_if.c | |
parent | e28a20a2d9d049cd6312e218a7646593bbc43431 (diff) |
sysmobts: Clarify msgb ownership / fix memory leaks
Diffstat (limited to 'src/osmo-bts-sysmo/l1_if.c')
-rw-r--r-- | src/osmo-bts-sysmo/l1_if.c | 51 |
1 files changed, 30 insertions, 21 deletions
diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c index 1b21df95..d80e2430 100644 --- a/src/osmo-bts-sysmo/l1_if.c +++ b/src/osmo-bts-sysmo/l1_if.c @@ -390,6 +390,7 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, LOGP(DL1C, LOGL_NOTICE, "unknown prim %d op %d " "chan_nr %d link_id %d\n", l1sap->oph.primitive, l1sap->oph.operation, chan_nr, link_id); + msgb_free(l1msg); return -EINVAL; } @@ -412,13 +413,10 @@ static int ph_data_req(struct gsm_bts_trx *trx, struct msgb *msg, empty_req_from_l1sap(l1p, fl1, u8Tn, u32Fn, sapi, subCh, u8BlockNbr); } - /* free the msgb holding the L1SAP primitive */ - msgb_free(msg); - /* send message to DSP's queue */ if (osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], l1msg) != 0) { LOGP(DL1P, LOGL_ERROR, "MQ_L1_WRITE queue full. Dropping msg.\n"); - msgb_free(msg); + msgb_free(l1msg); } return 0; @@ -490,7 +488,6 @@ static int ph_tch_req(struct gsm_bts_trx *trx, struct msgb *msg, /* send message to DSP's queue */ osmo_wqueue_enqueue(&fl1->write_q[MQ_L1_WRITE], nmsg); - msgb_free(msg); return 0; } @@ -539,7 +536,6 @@ static int mph_info_req(struct gsm_bts_trx *trx, struct msgb *msg, l1if_rsl_deact_sacch(lchan); else l1if_rsl_chan_rel(lchan); - msgb_free(msg); break; default: LOGP(DL1C, LOGL_NOTICE, "unknown MPH-INFO.req %d\n", @@ -556,6 +552,8 @@ int bts_model_l1sap_down(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap) struct msgb *msg = l1sap->oph.msg; int rc = 0; + /* called functions MUST NOT take ownership of msgb, as it is + * free()d below */ switch (OSMO_PRIM_HDR(&l1sap->oph)) { case OSMO_PRIM(PRIM_PH_DATA, PRIM_OP_REQUEST): rc = ph_data_req(trx, msg, l1sap); @@ -572,13 +570,14 @@ int bts_model_l1sap_down(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap) rc = -EINVAL; } - if (rc) - msgb_free(msg); + msgb_free(msg); + return rc; } static int handle_mph_time_ind(struct femtol1_hdl *fl1, - GsmL1_MphTimeInd_t *time_ind) + GsmL1_MphTimeInd_t *time_ind, + struct msgb *msg) { struct gsm_bts_trx *trx = fl1->priv; struct gsm_bts *bts = trx->bts; @@ -601,6 +600,8 @@ static int handle_mph_time_ind(struct femtol1_hdl *fl1, l1sap.u.info.type = PRIM_INFO_TIME; l1sap.u.info.u.time_ind.fn = fn; + msgb_free(msg); + return l1sap_up(trx, &l1sap); } @@ -725,6 +726,8 @@ static int handle_ph_readytosend_ind(struct femtol1_hdl *fl1, link_id = 0x40; else link_id = 0; + /* recycle the msgb and use it for the L1 primitive, + * which means that we (or our caller) must not free it */ rc = msgb_trim(l1p_msg, sizeof(*l1sap)); if (rc < 0) MSGB_ABORT(l1p_msg, "No room for primitive\n"); @@ -791,6 +794,8 @@ tx: msgb_free(resp_msg); } + /* free the msgb, as we have not handed it to l1sap and thus + * need to release its memory */ msgb_free(l1p_msg); return 0; @@ -821,6 +826,8 @@ static int process_meas_res(struct gsm_bts_trx *trx, uint8_t chan_nr, l1sap.u.info.u.meas_ind.ber10k = (unsigned int) (m->fBer * 100); l1sap.u.info.u.meas_ind.inv_rssi = (uint8_t) (m->fRssi * -1); + /* l1sap wants to take msgb ownership. However, as there is no + * msg, it will msgb_free(l1sap.oph.msg == NULL) */ return l1sap_up(trx, &l1sap); } @@ -955,30 +962,28 @@ static int l1if_handle_ind(struct femtol1_hdl *fl1, struct msgb *msg) GsmL1_Prim_t *l1p = msgb_l1prim(msg); int rc = 0; + /* all the below called functions must take ownership of the msgb */ switch (l1p->id) { case GsmL1_PrimId_MphTimeInd: - rc = handle_mph_time_ind(fl1, &l1p->u.mphTimeInd); + rc = handle_mph_time_ind(fl1, &l1p->u.mphTimeInd, msg); break; case GsmL1_PrimId_MphSyncInd: break; case GsmL1_PrimId_PhConnectInd: break; case GsmL1_PrimId_PhReadyToSendInd: - return handle_ph_readytosend_ind(fl1, &l1p->u.phReadyToSendInd, + rc = handle_ph_readytosend_ind(fl1, &l1p->u.phReadyToSendInd, msg); case GsmL1_PrimId_PhDataInd: - return handle_ph_data_ind(fl1, &l1p->u.phDataInd, msg); + rc = handle_ph_data_ind(fl1, &l1p->u.phDataInd, msg); + break; case GsmL1_PrimId_PhRaInd: - return handle_ph_ra_ind(fl1, &l1p->u.phRaInd, msg); + rc = handle_ph_ra_ind(fl1, &l1p->u.phRaInd, msg); break; default: break; } - /* Special return value '1' means: do not free */ - if (rc != 1) - msgb_free(msg); - return rc; } @@ -1012,9 +1017,11 @@ int l1if_handle_l1prim(int wq, struct femtol1_hdl *fl1h, struct msgb *msg) llist_for_each_entry(wlc, &fl1h->wlc_list, list) { if (is_prim_compat(l1p, wlc)) { llist_del(&wlc->list); - if (wlc->cb) + if (wlc->cb) { + /* call-back function must take + * ownership of msgb */ rc = wlc->cb(fl1h->priv, msg, wlc->cb_data); - else { + } else { rc = 0; msgb_free(msg); } @@ -1042,9 +1049,11 @@ int l1if_handle_sysprim(struct femtol1_hdl *fl1h, struct msgb *msg) * sending the same primitive */ if (wlc->is_sys_prim && sysp->id == wlc->conf_prim_id) { llist_del(&wlc->list); - if (wlc->cb) + if (wlc->cb) { + /* call-back function must take + * ownership of msgb */ rc = wlc->cb(fl1h->priv, msg, wlc->cb_data); - else { + } else { rc = 0; msgb_free(msg); } |