Change in openbsc[master]: bsc_nat: Fix crash (double-free) in forward_sccp_to_msc
Pau Espin Pedrol
gerrit-no-reply at lists.osmocom.org
Wed May 1 21:18:52 UTC 2019
Pau Espin Pedrol has uploaded this change for review. ( https://gerrit.osmocom.org/13837
Change subject: bsc_nat: Fix crash (double-free) in forward_sccp_to_msc
......................................................................
bsc_nat: Fix crash (double-free) in forward_sccp_to_msc
In bsc_nat_parse(), parsed is allocated this way:
"""parsed = talloc_zero(msg, struct bsc_nat_parsed);"""
So parsed is a child of msg, and so it's freed when msg is freed.
Since libosmocore c7f52c4c84d6a8898048738c4db9266289c40b45,
osmo_wqueue_enqueue() correctly detects queue full and returns an error,
and then queue_for_msc() calls msgb_free(). Code in osmo-bsc-nat was
probably written before that change in behavior, so that's why probably
the bug was not hit before.
Related: SYS#4548
Change-Id: I209d3e2d809a67915ec43c874e68f7f746a565f0
---
M openbsc/src/osmo-bsc_nat/bsc_nat.c
1 file changed, 11 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/37/13837/1
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index 670b0be..fb34aa7 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -108,19 +108,23 @@
return NULL;
}
-static void queue_for_msc(struct bsc_msc_connection *con, struct msgb *msg)
+static int queue_for_msc(struct bsc_msc_connection *con, struct msgb *msg)
{
+ int rc;
if (!con) {
LOGP(DLINP, LOGL_ERROR, "No MSC Connection assigned. Check your code.\n");
msgb_free(msg);
- return;
+ return -EINVAL;
}
-
- if (osmo_wqueue_enqueue(&con->write_queue, msg) != 0) {
+ rc = osmo_wqueue_enqueue(&con->write_queue, msg);
+ if (rc != 0) {
LOGP(DLINP, LOGL_ERROR, "Failed to enqueue the write.\n");
msgb_free(msg);
+ return rc;
}
+
+ return 0;
}
static void send_reset_ack(struct bsc_connection *bsc)
@@ -1277,9 +1281,10 @@
}
/* send the non-filtered but maybe modified msg */
- queue_for_msc(con_msc, msg);
- if (parsed)
+ if (queue_for_msc(con_msc, msg) == 0 && parsed)
talloc_free(parsed);
+ /* else: enqueue error, msg has been freed, and so child parsed is freed too */
+
return 0;
exit:
--
To view, visit https://gerrit.osmocom.org/13837
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I209d3e2d809a67915ec43c874e68f7f746a565f0
Gerrit-Change-Number: 13837
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190501/7224709f/attachment.html>
More information about the gerrit-log
mailing list