[PATCH] osmo-bts[master]: l1sap: Validate incoming RTP payload, drop bw-efficient AMR

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Pau Espin Pedrol gerrit-no-reply at lists.osmocom.org
Fri Feb 9 12:16:07 UTC 2018


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/6351

to look at the new patch set (#2).

l1sap: Validate incoming RTP payload, drop bw-efficient AMR

A recurrent kernel crash in sysmobts (several kernel versions)
corrupting kernel memory in random places has been investigated and
reproduced by placing a call against an MSC sending RTP
with bandwidth-efficient AMR payload to osmo-bts-sysmo.
The osmo-bts-sysmo in turn sends the payload to the femtobts related
kernel modules via a msgq, which most probably fail to handle correctly
this bw-efficient AMR payload and corrupt the kernel memory.

First approach was to drop the bw-efficient AMR payloads lower in the
stack in sysmo specific code (l1if_tch_encode), but as there's no bts
model in osmo-bts actually supporting bw-efficient AMR, let's drop it
early in the incoming path for all models to avoid further problems.

Related: SYS#4063

Change-Id: If0c9233c628c724de4ab74e58e3e2affac79e6d0
---
M src/common/l1sap.c
1 file changed, 45 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/51/6351/2

diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index e181a73..1deee83 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -819,6 +819,47 @@
 	return 0;
 }
 
+static bool rtppayload_is_octet_aligned(const uint8_t *rtp_pl, uint8_t payload_len)
+{
+	/*
+	 * Logic: If 1st bit padding is not zero, packet is either:
+	 * - bandwidth-efficient AMR payload.
+	 * - malformed packet.
+	 * However, Bandwidth-efficient AMR 4,75 frame last in payload(F=0, FT=0)
+	 * with 4th,5ht,6th AMR payload to 0 matches padding==0.
+	 * Furthermore, both AMR 4,75 bw-efficient and octet alignment are 14 bytes long (AMR 4,75 encodes 95b):
+	 * bw-efficient: 95b, + 4b hdr + 6b ToC = 105b, + padding = 112b = 14B.
+	 * octet-aligned: 1B hdr + 1B ToC + 95b = 111b, + padding = 112b = 14B.
+	 * We cannot use other fields to match since they are inside the AMR
+	 * payload bits which are unknown.
+	 * As a result, this function may return false positive (true) for some AMR
+	 * 4,75 AMR frames, but given the length, CMR and FT read is the same as a
+	 * consequence, the damage in here is harmless other than being unable to
+	 * decode the audio at the other side.
+	 */
+	#define AMR_PADDING1(rtp_pl) (rtp_pl[0] & 0x0f)
+	#define AMR_PADDING2(rtp_pl) (rtp_pl[1] & 0x03)
+
+	if(payload_len < 2 || AMR_PADDING1(rtp_pl) || AMR_PADDING2(rtp_pl))
+		return false;
+
+	return true;
+}
+
+static bool rtppayload_is_valid(struct gsm_lchan *lchan, struct msgb *resp_msg)
+{
+	/* Avoid sending bw-efficient AMR to lower layers, most bts models
+	 * don't support it. */
+	if(lchan->tch_mode == GSM48_CMODE_SPEECH_AMR &&
+		!rtppayload_is_octet_aligned(resp_msg->data, resp_msg->len)) {
+		LOGP(DL1P, LOGL_NOTICE,
+			"%s RTP->L1: Dropping unexpected AMR encoding (bw-efficient?) %s\n",
+			gsm_lchan_name(lchan), osmo_hexdump(resp_msg->data, resp_msg->len));
+		return false;
+	}
+	return true;
+}
+
 /* TCH-RTS-IND prim recevied from bts model */
 static int l1sap_tch_rts_ind(struct gsm_bts_trx *trx,
 	struct osmo_phsap_prim *l1sap, struct ph_tch_param *rts_ind)
@@ -860,6 +901,10 @@
 		LOGP(DL1P, LOGL_DEBUG, "%s DL TCH Tx queue underrun\n",
 			gsm_lchan_name(lchan));
 		resp_l1sap = &empty_l1sap;
+	} else if(!rtppayload_is_valid(lchan, resp_msg)) {
+		msgb_free(resp_msg);
+		resp_msg = NULL;
+		resp_l1sap = &empty_l1sap;
 	} else {
 		/* Obtain RTP header Marker bit from control buffer */
 		marker = rtpmsg_marker_bit(resp_msg);

-- 
To view, visit https://gerrit.osmocom.org/6351
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If0c9233c628c724de4ab74e58e3e2affac79e6d0
Gerrit-PatchSet: 2
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>



More information about the gerrit-log mailing list