From bf42dd6b23d8b3ad486bfebc48717860b7b996da Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 28 Aug 2014 19:24:00 +0200 Subject: osmux: fix leaks in osmux_xfrm_input() error path If it is not possible to put the RTP message into the batch in case that: 1) The message is malformed. 2) The message is duplicated. 3) OOM. 4) The batch is already full. Osmux releases the messages and gets back to the upper layer with an OK to give another chance to follow up RTP messages. This patch also fixes a use-after-free that was possible when the batch was full. The message was released and osmux_batch_add() was returning 0 osmux_xfrm_input(), which resulted in accessing the released message when updating the statistics. --- src/osmux.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/osmux.c b/src/osmux.c index f6f211d..0a7fa6e 100644 --- a/src/osmux.c +++ b/src/osmux.c @@ -205,7 +205,6 @@ static int osmux_batch_enqueue(struct msgb *msg, struct batch_list_node *node) LOGP(DLMIB, LOGL_ERROR, "too many messages for this RTP " "ssrc=%u\n", rtph->ssrc); - msgb_free(msg); return -1; } @@ -437,8 +436,10 @@ static void osmux_replay_lost_packets(struct batch_list_node *node, DELTA_RTP_TIMESTAMP); /* No more room in this batch, skip padding with more clones */ - if (osmux_batch_enqueue(clone, node) < 0) + if (osmux_batch_enqueue(clone, node) < 0) { + msgb_free(clone); break; + } LOGP(DLMIB, LOGL_ERROR, "adding cloned RTP\n"); } @@ -460,7 +461,7 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg, amr_payload_len = osmux_rtp_amr_payload_len(msg, rtph); if (amr_payload_len < 0) - return 0; + return -1; /* First check if there is room for this message in the batch */ bytes += amr_payload_len; @@ -481,14 +482,14 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg, llist_for_each_entry(cur, &node->list, list) { struct rtp_hdr *rtph2 = osmo_rtp_get_hdr(cur); if (rtph2 == NULL) - return 0; + return -1; /* Already exists message with this sequence, skip */ if (rtph2->sequence == rtph->sequence) { LOGP(DLMIB, LOGL_ERROR, "already exists " "message with seq=%u, skip it\n", rtph->sequence); - return 0; + return -1; } } /* Handle RTP packet loss scenario */ @@ -498,7 +499,7 @@ osmux_batch_add(struct osmux_batch *batch, struct msgb *msg, /* This is the first message with that ssrc we've seen */ node = talloc_zero(osmux_ctx, struct batch_list_node); if (node == NULL) - return 0; + return -1; node->ccid = ccid; INIT_LLIST_HEAD(&node->list); @@ -560,8 +561,14 @@ int osmux_xfrm_input(struct osmux_in_handle *h, struct msgb *msg, int ccid) /* Add this RTP to the OSMUX batch */ ret = osmux_batch_add(batch, msg, rtph, ccid); - if (ret < 0) + if (ret < 0) { + /* Cannot put this message into the batch. + * Malformed, duplicated, OOM. Drop it and tell + * the upper layer that we have digest it. + */ + msgb_free(msg); return 0; + } h->stats.input_rtp_msgs++; h->stats.input_rtp_bytes += msg->len; -- cgit v1.2.3