When closing down a cmux object, the address sanitizer detects a
use-after-free in gatmux.c (see below).
Avoid this by taking a reference to the mux object during the processing
in received_data().
ofonod[3640549]: ../git/plugins/quectel.c:cfun_disable() 0x610000000b40
ofonod[3640549]: ../git/plugins/quectel.c:close_serial() 0x610000000b40
ofonod[3640549]: ../git/plugins/quectel.c:close_mux() 0x610000000b40
ofonod[3640549]: ../git/examples/emulator.c:powered_watch() Removing modem
0x610000000b40 from the list
ofonod[3640549]: ../git/examples/emulator.c:powered_watch() Removing server
watch: 106
ofonod[3640549]: ../git/src/modem.c:modem_change_state() old state: 0, new
state: 0
=================================================================
==3640549==ERROR: AddressSanitizer: heap-use-after-free on address
0x62100073dd28 at pc 0x5566b6402a21 bp 0x7ffe7a2db0e0 sp 0x7ffe7a2db0d0
READ of size 8 at 0x62100073dd28 thread T0
#0 0x5566b6402a20 in debug ../git/gatchat/gatmux.c:109
#1 0x5566b6404bd7 in channel_close ../git/gatchat/gatmux.c:525
#2 0x7fa0516e44a6 in g_io_channel_shutdown
(/usr/lib/libglib-2.0.so.0+0x774a6)
#3 0x7fa0516e4644 in g_io_channel_unref (/usr/lib/libglib-2.0.so.0+0x77644)
#4 0x5566b64048a4 in watch_finalize ../git/gatchat/gatmux.c:474
#5 0x7fa0516d6f6f (/usr/lib/libglib-2.0.so.0+0x69f6f)
#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
#10 0x7fa0516d727e in g_main_context_dispatch
(/usr/lib/libglib-2.0.so.0+0x6a27e)
#11 0x7fa0516d91c0 (/usr/lib/libglib-2.0.so.0+0x6c1c0)
#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
#13 0x5566b6429b1b in main ../git/src/main.c:286
#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
#15 0x5566b62531ad in _start
(/home/martin/projects/ofono/x86/src/ofonod+0xfc1ad)
0x62100073dd28 is located 40 bytes inside of 4672-byte region
[0x62100073dd00,0x62100073ef40)
freed by thread T0 here:
#0 0x7fa0519256c0 in __interceptor_free
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
#1 0x5566b64052d7 in g_at_mux_unref ../git/gatchat/gatmux.c:645
#2 0x5566b63d6d19 in close_mux ../git/plugins/quectel.c:199
#3 0x5566b63d7047 in close_serial ../git/plugins/quectel.c:223
#4 0x5566b63db62a in cfun_disable ../git/plugins/quectel.c:1056
#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
#6 0x5566b63f701b in at_chat_handle_command_response
../git/gatchat/gatchat.c:521
#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
#13 0x7fa0516d727e in g_main_context_dispatch
(/usr/lib/libglib-2.0.so.0+0x6a27e)
previously allocated by thread T0 here:
#0 0x7fa051925ce8 in __interceptor_calloc
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
#1 0x5566b6405009 in g_at_mux_new ../git/gatchat/gatmux.c:606
#2 0x5566b6407f6b in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1165
#3 0x5566b63da9ba in cmux_cb ../git/plugins/quectel.c:882
#4 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
#5 0x5566b63f701b in at_chat_handle_command_response
../git/gatchat/gatchat.c:521
#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
#9 0x7fa0516d727e in g_main_context_dispatch
(/usr/lib/libglib-2.0.so.0+0x6a27e)
SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:109 in
debug
Shadow bytes around the buggy address:
0x0c42800dfb50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c42800dfb60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c42800dfb70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c42800dfb80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c42800dfb90: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c42800dfba0: fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd
0x0c42800dfbb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c42800dfbc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c42800dfbd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c42800dfbe0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c42800dfbf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
Shadow gap: cc
==3640549==ABORTING
---
Changes since v2:
* narrow the scope of the taken reference to the for loop
* defer removal of destroy notification callback to separate patch
gatchat/gatmux.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
index 757a5123..d50e5361 100644
--- a/gatchat/gatmux.c
+++ b/gatchat/gatmux.c
@@ -231,6 +231,7 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
int i;
GIOStatus status;
gsize bytes_read;
+ gboolean buffer_full = FALSE;
if (cond & G_IO_NVAL)
return FALSE;
@@ -255,6 +256,8 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
if (mux->buf_used > 0)
memmove(mux->buf, mux->buf + nread, mux->buf_used);
+ g_at_mux_ref(mux);
+
for (i = 1; i <= MAX_CHANNELS; i++) {
int offset = i / 8;
int bit = i % 8;
@@ -267,6 +270,10 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
dispatch_sources(mux->dlcs[i-1], G_IO_IN);
}
+
+ buffer_full = mux->buf_used == sizeof(mux->buf);
+
+ g_at_mux_unref(mux);
}
if (cond & (G_IO_HUP | G_IO_ERR))
@@ -275,7 +282,7 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
return FALSE;
- if (mux->buf_used == sizeof(mux->buf))
+ if (buffer_full)
return FALSE;
return TRUE;
--
2.23.0
_______________________________________________
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
