Project

General

Profile

Actions

Bug #3950

closed

Avoid NULL talloc context use in mgcp_conn_alloc

Added by pespin almost 5 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
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.

Actions #1

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.

Actions #2

Updated by pespin over 4 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

Merged, closing.

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)