[PATCH] osmo-bts[master]: l1sap: Avoid assumption that l1sap is at head of msgb

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
Wed Mar 21 18:57:46 UTC 2018


Review at  https://gerrit.osmocom.org/7429

l1sap: Avoid assumption that l1sap is at head of msgb

This assumption used while sending the rx data to gsmtap in l1sap_up was
making osmo-bts-virtual crash, since that bts model is allocating the
l1sap in the stack rather than inside the msgb.
Instead, let's use the assumption that l2h is set correctly in msgb by
the bts model lower layer.

crash report:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6234ec3 in __memmove_sse2_unaligned_erms () from /usr/lib/libc.so.6
(gdb) bt
0  0x00007ffff6234ec3 in __memmove_sse2_unaligned_erms () from /usr/lib/libc.so.6
1  0x00007ffff6dbf4c8 in gsmtap_makemsg_ex (type=<optimized out>, arfcn=arfcn at entry=17255, ts=ts at entry=6 '\006',
    chan_type=<optimized out>, ss=ss at entry=0 '\000', fn=fn at entry=11249, signal_dbm=0 '\000', snr=0 '\000',
    data=0x5555557d5b50 "", len=4294967263) at libosmocore/src/gsmtap_util.c:179
2  0x00007ffff6dbf6d8 in gsmtap_send_ex (gti=0x555555877f10, type=type at entry=1 '\001', arfcn=arfcn at entry=17255,
    ts=ts at entry=6 '\006', chan_type=<optimized out>, ss=<optimized out>, fn=11249, signal_dbm=0 '\000', snr=0 '\000',
    data=0x5555557d5b50 "", len=4294967263) at libosmocore/src/gsmtap_util.c:311
3  0x00007ffff6dbf765 in gsmtap_send (gti=<optimized out>, arfcn=arfcn at entry=17255, ts=ts at entry=6 '\006',
    chan_type=<optimized out>, ss=<optimized out>, fn=fn at entry=11249, signal_dbm=0 '\000', snr=0 '\000',
    data=0x5555557d5b50 "", len=4294967263) at libosmocore/src/gsmtap_util.c:330
4  0x0000555555573571 in to_gsmtap (trx=0x7ffff7ef8070, l1sap=0x7fffffffde80)
    at osmo-bts/src/common/l1sap.c:397
5  0x0000555555573b9c in l1sap_up (trx=0x7ffff7ef8070, l1sap=l1sap at entry=0x7fffffffde80)
    at osmo-bts/src/common/l1sap.c:1285
6  0x000055555555ec06 in virt_um_rcv_cb (vui=<optimized out>, msg=<optimized out>)
    at osmo-bts/src/osmo-bts-virtual/l1_if.c:170
7  0x000055555555f5c6 in virt_um_fd_cb (ofd=0x55555587cc30, what=<optimized out>)
    at osmo-bts/src/osmo-bts-virtual/virtual_um.c:50
8  0x00007ffff6db6991 in osmo_fd_disp_fds (_eset=0x7fffffffe090, _wset=0x7fffffffe010, _rset=0x7fffffffdf90)
    at libosmocore/src/select.c:216
9  osmo_select_main (polling=polling at entry=0) at libosmocore/src/select.c:256
10 0x0000555555576fbc in bts_main (argc=5, argv=0x7fffffffe288)
    at osmo-bts/src/common/main.c:364
11 0x00007ffff61b5f4a in __libc_start_main () from /usr/lib/libc.so.6
12 0x000055555555c4ca in _start ()

In the old code when the sizeof(osmo_phsap_prim) was being substracted
it resulted on a negative len which later was casted to unsigned int and
became a really big number.

Fixes: OS#3092

Change-Id: I51a880328497673a06d153bfb76c428265b8cbb8
---
M src/common/l1sap.c
1 file changed, 15 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/29/7429/1

diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 0a229e1..e7cef4e 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -235,14 +235,14 @@
 
 /* send primitive as gsmtap */
 static int gsmtap_ph_data(struct osmo_phsap_prim *l1sap, uint8_t *chan_type,
-			  uint8_t *ss, uint32_t fn, uint8_t **data, int *len,
+			  uint8_t *ss, uint32_t fn, uint8_t **data, unsigned int *len,
 			  uint8_t num_agch)
 {
 	struct msgb *msg = l1sap->oph.msg;
 	uint8_t chan_nr, link_id;
 
-	*data = msg->data + sizeof(struct osmo_phsap_prim);
-	*len = msg->len - sizeof(struct osmo_phsap_prim);
+	*data = msgb_l2(msg);
+	*len = msgb_l2len(msg);
 	chan_nr = l1sap->u.data.chan_nr;
 	link_id = l1sap->u.data.link_id;
 
@@ -276,18 +276,18 @@
 }
 
 static int gsmtap_pdch(struct osmo_phsap_prim *l1sap, uint8_t *chan_type,
-		       uint8_t *ss, uint32_t fn, uint8_t **data, int *len)
+		       uint8_t *ss, uint32_t fn, uint8_t **data, unsigned int *len)
 {
 	struct msgb *msg = l1sap->oph.msg;
 
-	*data = msg->data + sizeof(struct osmo_phsap_prim);
-	*len = msg->len - sizeof(struct osmo_phsap_prim);
+	*data = msgb_l2(msg);
+	*len = msgb_l2len(msg);
 
 	if (L1SAP_IS_PTCCH(fn)) {
 		*chan_type = GSMTAP_CHANNEL_PTCCH;
 		*ss = L1SAP_FN2PTCCHBLOCK(fn);
-		if (l1sap->oph.primitive
-				== PRIM_OP_INDICATION) {
+		if (l1sap->oph.primitive == PRIM_OP_INDICATION) {
+			OSMO_ASSERT(len > 0);
 			if ((*data[0]) == 7)
 				return -EINVAL;
 			(*data)++;
@@ -300,7 +300,7 @@
 }
 
 static int gsmtap_ph_rach(struct osmo_phsap_prim *l1sap, uint8_t *chan_type,
-	uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, int *len)
+	uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, unsigned int *len)
 {
 	uint8_t chan_nr;
 
@@ -345,7 +345,7 @@
 static int to_gsmtap(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap)
 {
 	uint8_t *data;
-	int len;
+	unsigned int len;
 	uint8_t chan_type = 0, tn = 0, ss = 0;
 	uint32_t fn;
 	uint16_t uplink = GSMTAP_ARFCN_F_UPLINK;
@@ -1262,7 +1262,11 @@
 	return 0;
 }
 
-/* any L1 prim received from bts model, takes ownership of the msgb */
+/* Process any L1 prim received from bts model.
+ *
+ * This function takes ownership of the msgb.
+ * If l1sap contains a msgb, it assumes that msgb->l2h was set by lower layer.
+ */
 int l1sap_up(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap)
 {
 	struct msgb *msg = l1sap->oph.msg;

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I51a880328497673a06d153bfb76c428265b8cbb8
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>



More information about the gerrit-log mailing list