Change in osmo-bts[master]: common/oml.c: fix total length calculation in cleanup_attr_msg()

Vadim Yanitskiy gerrit-no-reply at lists.osmocom.org
Sun Apr 21 09:06:58 UTC 2019


Vadim Yanitskiy has submitted this change and it was merged. ( https://gerrit.osmocom.org/13707 )

Change subject: common/oml.c: fix total length calculation in cleanup_attr_msg()
......................................................................

common/oml.c: fix total length calculation in cleanup_attr_msg()

Both callers of cleanup_attr_msg(), i.e. handle_attrs_trx() and
handle_attrs_bts(), always pass out_offset >= 1, so the length
of the unsupported attributes counter is already accounted.

Otherwise, both callers would copy an additional garbage byte
from uninitialized memory. Discovered using Valgrind:

DOML DEBUG oml.c:539 OC=BTS(01) INST=(ff,ff,ff) Rx GET ATTR
DOML INFO oml.c:265 BTS Tx Get Attribute Response
==25467== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==25467==    at 0x623E0BD: send (send.c:27)
==25467==    by 0x5685846: __handle_ts1_write (ipaccess.c:358)
==25467==    by 0x5683F8B: ipa_client_write (ipa.c:79)
==25467==    by 0x5683F8B: ipa_client_fd_cb (ipa.c:140)
==25467==    by 0x5F1DC23: osmo_fd_disp_fds (select.c:223)
==25467==    by 0x5F1DC23: osmo_select_main (select.c:263)
==25467==    by 0x42980B: bts_main (main.c:354)
==25467==    by 0x6160F44: (below main) (libc-start.c:287)
==25467==  Address 0x7d83895 is 23,669 bytes inside a block of size 102,528 alloc'd
==25467==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25467==    by 0x589A6B4: talloc_pool (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.1.5)
==25467==    by 0x5F1E28B: msgb_talloc_ctx_init (msgb.c:316)
==25467==    by 0x4293D0: bts_main (main.c:234)
==25467==    by 0x6160F44: (below main) (libc-start.c:287)
==25467==  Uninitialised value was created by a stack allocation
==25467==    at 0x415FE5: oml_tx_attr_resp (oml.c:259)
==25467==    by 0x415FE5: oml_rx_get_attr (oml.c:561)
==25467==

Change-Id: Ic7c2c4e54e9f99b60aaf70604044933978be945c
Related: OS#3938
---
M src/common/oml.c
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved
  Vadim Yanitskiy: Looks good to me, but someone else must approve



diff --git a/src/common/oml.c b/src/common/oml.c
index 307e8d9..de7a0e7 100644
--- a/src/common/oml.c
+++ b/src/common/oml.c
@@ -188,7 +188,7 @@
 		msgb_free(msg);
 	}
 
-	return len + out_offset + 1;
+	return len + out_offset;
 }
 
 static inline int handle_attrs_trx(uint8_t *out, const struct gsm_bts_trx *trx, const uint8_t *attr, uint16_t attr_len)

-- 
To view, visit https://gerrit.osmocom.org/13707
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic7c2c4e54e9f99b60aaf70604044933978be945c
Gerrit-Change-Number: 13707
Gerrit-PatchSet: 2
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190421/25dbb037/attachment.html>


More information about the gerrit-log mailing list