Actions
Bug #3950
closedAvoid NULL talloc context use in mgcp_conn_alloc
Start date:
04/23/2019
Due date:
% Done:
100%
Spec Reference:
Description
I spotted osmo-mgw is allocating connections on the NULL talloc context. We usually want to avoid that, so I tried following patch:
commit f454fe98d01d12f952de192156078b3a877419f9 (HEAD -> pespin/osmux) Author: Pau Espin Pedrol <pespin@sysmocom.de> Date: Tue Apr 23 13:30:32 2019 +0200 mgcp_protocol: Avoid NULL talloc ctx during mgcp_conn_alloc call Change-Id: Iad53d18a5f3f09d726dbeeaca46e920ad97b7704 diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index ac6b4ea5..c48bc457 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -907,7 +907,7 @@ mgcp_header_done: endp->callid = talloc_strdup(tcfg->endpoints, callid); snprintf(conn_name, sizeof(conn_name), "%s", callid); - _conn = mgcp_conn_alloc(NULL, endp, MGCP_CONN_TYPE_RTP, conn_name); + _conn = mgcp_conn_alloc(tcfg->endpoints, endp, MGCP_CONN_TYPE_RTP, conn_name); if (!_conn) { LOGP(DLMGCP, LOGL_ERROR, "CRCX: endpoint:0x%x unable to allocate RTP connection\n",
But then this unit test fails:
2: mgcp FAILED (testsuite.at:14)
Further lookup shows a head-use-after-free catched by ASan:
<0010> /home/pespin/dev/sysmocom/git/osmo-mgw/src/libosmo-mgcp/mgcp_protocol.c:799 CRCX: creating new connection ... <0010> /home/pespin/dev/sysmocom/git/osmo-mgw/src/libosmo-mgcp/mgcp_msg.c:394 Wrong MGCP option format: 'X' on 0x7 <0010> /home/pespin/dev/sysmocom/git/osmo-mgw/src/libosmo-mgcp/mgcp_sdp.c:382 Got media info via SDP: port:5904, addr:123.12.12.123, duration:20, payload-types:111=AMR Policy CB got state 1 on endpoint 0x7 Dummy packet to 0x7b0c0c7b:5904, msg length 1 23 Dummy packet to 0x7b0c0c7b:5905, msg length 1 23 <0010> /home/pespin/dev/sysmocom/git/osmo-mgw/src/libosmo-mgcp/mgcp_protocol.c:1038 CRCX: endpoint:0x7 connection successfully created ================================================================= ==31947==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000001ce0 at pc 0x7f56db767f91 bp 0x7ffcbe587110 sp 0x7ffcbe587100 WRITE of size 8 at 0x617000001ce0 thread T0 #0 0x7f56db767f90 in __llist_del /home/pespin/dev/sysmocom/git/libosmocore/include/osmocom/core/linuxlist.h:118 #1 0x7f56db768076 in llist_del /home/pespin/dev/sysmocom/git/libosmocore/include/osmocom/core/linuxlist.h:129 #2 0x7f56db769dc6 in rate_ctr_group_free /home/pespin/dev/sysmocom/git/libosmocore/src/rate_ctr.c:262 #3 0x55a28114fcbb in free_rate_counter_group /home/pespin/dev/sysmocom/git/osmo-mgw/src/libosmo-mgcp/mgcp_protocol.c:1533 #4 0x7f56db630875 (/usr/lib/libtalloc.so.2+0xb875) #5 0x7f56db62afb7 in _talloc_free (/usr/lib/libtalloc.so.2+0x5fb7) #6 0x55a28113278a in test_messages /home/pespin/dev/sysmocom/git/osmo-mgw/tests/mgcp/mgcp_test.c:859 #7 0x55a28113cfab in main /home/pespin/dev/sysmocom/git/osmo-mgw/tests/mgcp/mgcp_test.c:1830 #8 0x7f56da7d2222 in __libc_start_main (/usr/lib/libc.so.6+0x24222) #9 0x55a28112e86d in _start (/home/pespin/dev/sysmocom/build/new/tmpdir/osmo-mgw/tests/mgcp/mgcp_test+0x9286d) 0x617000001ce0 is located 96 bytes inside of 688-byte region [0x617000001c80,0x617000001f30) freed by thread T0 here: #0 0x7f56dc209c19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66 #1 0x7f56db630323 (/usr/lib/libtalloc.so.2+0xb323) previously allocated by thread T0 here: #0 0x7f56dc20a019 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86 #1 0x7f56db62de11 in _talloc_zero (/usr/lib/libtalloc.so.2+0x8e11) SUMMARY: AddressSanitizer: heap-use-after-free /home/pespin/dev/sysmocom/git/libosmocore/include/osmocom/core/linuxlist.h:118 in __llist_del Shadow bytes around the buggy address: 0x0c2e7fff8340: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff8350: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff8360: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff8370: fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa 0x0c2e7fff8380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c2e7fff8390: fd fd fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd 0x0c2e7fff83a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff83b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff83c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff83d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e7fff83e0: fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==31947==ABORTING
Then instead, of regular output it prints:
(response does not contain a connectio
Need to check why this change makes it fail, probably a bug in the test.
Updated by pespin over 4 years ago
- Status changed from New to Feedback
- % Done changed from 0 to 90
crash and NULL ctx used fixed here:
remote: https://gerrit.osmocom.org/c/osmo-mgw/+/15580 mgcp_test: Correctly release all endpoints allocated
remote: https://gerrit.osmocom.org/c/osmo-mgw/+/15581 mgw: Allocate mgcp_conn instance under tcfg->endpoints
Once merged the ticket can be closed.
Updated by pespin over 4 years ago
- Status changed from Feedback to Resolved
- % Done changed from 90 to 100
Merged, closing.
Actions