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_sndcp.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_sndcp.c')
-rw-r--r-- | openbsc/src/gprs/gprs_sndcp.c | 9 |
1 files changed, 6 insertions, 3 deletions
diff --git a/openbsc/src/gprs/gprs_sndcp.c b/openbsc/src/gprs/gprs_sndcp.c index 98da4aea8..46d779b4c 100644 --- a/openbsc/src/gprs/gprs_sndcp.c +++ b/openbsc/src/gprs/gprs_sndcp.c @@ -358,8 +358,10 @@ static int sndcp_send_ud_frag(struct sndcp_frag_state *fs) fmsg = msgb_alloc_headroom(fs->sne->lle->params.n201_u+256, 128, "SNDCP Frag"); - if (!fmsg) + if (!fmsg) { + msgb_free(fs->msg); return -ENOMEM; + } /* make sure lower layers route the fragment like the original */ msgb_tlli(fmsg) = msgb_tlli(fs->msg); @@ -416,9 +418,9 @@ static int sndcp_send_ud_frag(struct sndcp_frag_state *fs) sch->more = more; rc = gprs_llc_tx_ui(fmsg, lle->sapi, 0, fs->mmcontext); + /* abort in case of error, do not advance frag_nr / next_byte */ if (rc < 0) { - /* abort in case of error, do not advance frag_nr / next_byte */ - msgb_free(fmsg); + msgb_free(fs->msg); return rc; } @@ -450,6 +452,7 @@ int sndcp_unitdata_req(struct msgb *msg, struct gprs_llc_lle *lle, uint8_t nsapi sne = gprs_sndcp_entity_by_lle(lle, nsapi); if (!sne) { LOGP(DSNDCP, LOGL_ERROR, "Cannot find SNDCP Entity\n"); + msgb_free(msg); return -EIO; } |