diff options
author | Neels Hofmeyr <neels@hofmeyr.de> | 2017-11-18 22:22:59 +0100 |
---|---|---|
committer | Neels Hofmeyr <nhofmeyr@sysmocom.de> | 2017-11-20 13:49:32 +0000 |
commit | b3fa355321de7e0b4f3b83afebba203aa579a7d4 (patch) | |
tree | 263889cb227ba38ccb8affb6e44b85106789fdad /src/libvlr | |
parent | 1a5bcd5c3b3c84dbd1bf99fe08eaab51370fbef9 (diff) |
vlr_gsupc_read_cb: fix use after free of GSUP msgb
osmo_gsup_decode() doesn't actually decode everything, it does leave quite a
number of pointers into the original msgb. Hence we must not deallocate the
gsup msgb before dispatching GSUP events.
Move msgb_free() to the bottom of vlr_gsupc_read_cb() and use rc and gotos to
early-exit if needed.
Change-Id: I16fc92dcf84e29fcf34712a2e8b0464ef08425ad
Diffstat (limited to 'src/libvlr')
-rw-r--r-- | src/libvlr/vlr.c | 15 |
1 files changed, 10 insertions, 5 deletions
diff --git a/src/libvlr/vlr.c b/src/libvlr/vlr.c index 07c31ee8a..4ef52da4e 100644 --- a/src/libvlr/vlr.c +++ b/src/libvlr/vlr.c @@ -804,12 +804,11 @@ int vlr_gsupc_read_cb(struct gsup_client *gsupc, struct msgb *msg) osmo_hexdump_nospc(msgb_l2(msg), msgb_l2len(msg))); rc = osmo_gsup_decode(msgb_l2(msg), msgb_l2len(msg), &gsup); - msgb_free(msg); if (rc < 0) { LOGP(DVLR, LOGL_ERROR, "decoding GSUP message fails with error '%s' (%d)\n", get_value_string(gsm48_gmm_cause_names, -rc), -rc); - return rc; + goto msgb_free_and_return; } if (!gsup.imsi[0]) { @@ -817,7 +816,8 @@ int vlr_gsupc_read_cb(struct gsup_client *gsupc, struct msgb *msg) if (OSMO_GSUP_IS_MSGT_REQUEST(gsup.message_type)) vlr_tx_gsup_error_reply(vlr, &gsup, GMM_CAUSE_INV_MAND_INFO); - return -GMM_CAUSE_INV_MAND_INFO; + rc = -GMM_CAUSE_INV_MAND_INFO; + goto msgb_free_and_return; } vsub = vlr_subscr_find_by_imsi(vlr, gsup.imsi); @@ -825,9 +825,11 @@ int vlr_gsupc_read_cb(struct gsup_client *gsupc, struct msgb *msg) switch (gsup.message_type) { case OSMO_GSUP_MSGT_PURGE_MS_RESULT: case OSMO_GSUP_MSGT_PURGE_MS_ERROR: - return vlr_rx_gsup_purge_no_subscr(vlr, &gsup); + rc = vlr_rx_gsup_purge_no_subscr(vlr, &gsup); + goto msgb_free_and_return; default: - return vlr_rx_gsup_unknown_imsi(vlr, &gsup); + rc = vlr_rx_gsup_unknown_imsi(vlr, &gsup); + goto msgb_free_and_return; } } @@ -865,6 +867,9 @@ int vlr_gsupc_read_cb(struct gsup_client *gsupc, struct msgb *msg) } vlr_subscr_put(vsub); + +msgb_free_and_return: + msgb_free(msg); return rc; } |