diff options
author | Holger Hans Peter Freyther <holger@moiji-mobile.com> | 2014-10-10 17:35:54 +0200 |
---|---|---|
committer | Holger Hans Peter Freyther <holger@moiji-mobile.com> | 2014-10-10 17:43:40 +0200 |
commit | f9ffd1fa1811914ce6b19f1d17e7a908e550d358 (patch) | |
tree | 48ddf7b0c97525703ddd16361b8b3fafb6b2f2ea /openbsc/src/gprs/gprs_llc.c | |
parent | b4f0e8089d6ee18a282fb839f240ea65a3d89b63 (diff) |
sgsn: Prevent memory leak and double free
This has been re-produced using the "osmo-pcu emulator" code
and a ping to force segmented SNDCP messages. When the NS link
enters the DEAD/BLOCKED state the msgb would be freed twice.
Once inside gprs_ns_sendmsg and once by the caller. Based on the
return one can not see if the parameter has been deleted.
I changed libosmocore/libosmogb to always free the msgb in case
of an error on the way to gprs_ns_sendmsg. Catch up, avoid the
double free and fix some memory leaks. In case the sending fails
assume the entire segmented message is at end and free the
original input data.
This has been tested by posix suspending/resuming the emulator
process to have the GPRS-NS link go to dead/blocked to alive
and unblocked. The ping recovers and "SIGUSR1" to the SGSN does
not show active memory allocations.
The SGSN calls bssgp_tx_dl_ud at the lowest level and has the
following callchains. Most of them allocate the msgb and have
no early return and transfer ownership already:
<- gprs_llc_tx_u
<- gprs_llc_tx_ui
<- gsm48_gmm_sendmsg (all callers sane)
<- _tx_status
<- _tx_detach_req
<- gprs_llc_tx_xid (all callers sane)
<- sndcp_unitdata_req
<- sndcp_send_ud_frag
Diffstat (limited to 'openbsc/src/gprs/gprs_llc.c')
-rw-r--r-- | openbsc/src/gprs/gprs_llc.c | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/openbsc/src/gprs/gprs_llc.c b/openbsc/src/gprs/gprs_llc.c index cb810c4a2..c88d16269 100644 --- a/openbsc/src/gprs/gprs_llc.c +++ b/openbsc/src/gprs/gprs_llc.c @@ -382,6 +382,7 @@ int gprs_llc_tx_ui(struct msgb *msg, uint8_t sapi, int command, if (msg->len > lle->params.n201_u) { LOGP(DLLC, LOGL_ERROR, "Cannot Tx %u bytes (N201-U=%u)\n", msg->len, lle->params.n201_u); + msgb_free(msg); return -EFBIG; } @@ -439,6 +440,7 @@ int gprs_llc_tx_ui(struct msgb *msg, uint8_t sapi, int command, kc, iv, GPRS_CIPH_SGSN2MS); if (rc < 0) { LOGP(DLLC, LOGL_ERROR, "Error crypting UI frame: %d\n", rc); + msgb_free(msg); return rc; } |