Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Needs Testing] Bump Ofono to v1.24 #6

Merged
merged 49 commits into from
Sep 13, 2021
Merged

Conversation

PsychoGame
Copy link
Contributor

This pull request contains all cherry-picked commits between release 1.23, which is in use by Sailfish OS right now, and 1.24 in upstream Ofono. All cherry-picked commits were applied cleanly, without any manual intervention necessary, so I do not expect any real problems. At least It has already been verified that the package compiles properly. I can only test the package on a PinePhone, so if others will test it on their devices/builds please let me know if there are any regressions.

denkenz and others added 30 commits September 7, 2021 19:13
The product ID is added to the list of
modems to be detected by Ofono.
The gemalto plugin is used to handle the
ALS3 modem.
	ALS3, PLS8-E and PLS8-X have same vid/pid with same enumeration process
Force serial port opening options
Wait for modem to be ready to start
initializing it
Handle LTE
==14399== 16 bytes in 8 blocks are definitely lost in loss record 132 of 390
==14399==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==14399==    by 0x59E03D9: strndup (strndup.c:43)
==14399==    by 0x18277E: qmi_result_get_string (qmi.c:1794)
==14399==    by 0x184221: get_ids_cb (devinfo.c:129)
==14399==    by 0x18353B: service_send_callback (qmi.c:2286)
==14399==    by 0x18093C: handle_packet (qmi.c:831)
==14399==    by 0x180ADD: received_data (qmi.c:880)
==14399==    by 0x4E826A9: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.3)
==14399==    by 0x4E82A5F: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.3)
==14399==    by 0x4E82D81: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.3)
==14399==    by 0x201900: main (main.c:306)
==14399== 28 bytes in 4 blocks are definitely lost in loss record 151 of 390
==14399==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==14399==    by 0x209065: convert_gsm_to_utf8_with_lang (util.c:651)
==14399==    by 0x2091D1: convert_gsm_to_utf8 (util.c:690)
==14399==    by 0x22DDA7: ussd_decode (smsutil.c:4738)
==14399==    by 0x18BF71: qmi_ussd_request (ussd.c:233)
==14399==    by 0x2183EA: ussd_initiate (ussd.c:614)
==14399==    by 0x27B6C8: process_message (object.c:259)
==14399==    by 0x27D1CD: generic_message (object.c:1070)
==14399==    by 0x5170732: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.14.14)
==14399==    by 0x5161D83: dbus_connection_dispatch (in /lib/x86_64-linux-gnu/libdbus-1.so.3.14.14)
==14399==    by 0x27907C: message_dispatch (mainloop.c:72)
==14399==    by 0x4E826A9: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5000.3)
==2870== Conditional jump or move depends on uninitialised value(s)
==2870==    at 0x4C2ED31: __memcmp_sse4_1 (vg_replace_strmem.c:972)
==2870==    by 0x4F451A: sim_pin_retries_query_cb (sim.c:462)
==2870==    by 0x459BDD: query_pin_retries_cb (sim.c:544)
==2870==    by 0x45544A: service_send_callback (qmi.c:2143)
==2870==    by 0x452D00: handle_packet (qmi.c:815)
==2870==    by 0x452E85: received_data (qmi.c:863)
==2870==    by 0x508DB6C: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508DF47: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508E271: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x4C680B: main (main.c:256)
==2870==  Uninitialised value was created by a stack allocation
==2870==    at 0x459B1A: query_pin_retries_cb (sim.c:531)
==2870==
==2870== Conditional jump or move depends on uninitialised value(s)
==2870==    at 0x4F451D: sim_pin_retries_query_cb (sim.c:462)
==2870==    by 0x459BDD: query_pin_retries_cb (sim.c:544)
==2870==    by 0x45544A: service_send_callback (qmi.c:2143)
==2870==    by 0x452D00: handle_packet (qmi.c:815)
==2870==    by 0x452E85: received_data (qmi.c:863)
==2870==    by 0x508DB6C: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508DF47: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508E271: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x4C680B: main (main.c:256)
==2870==  Uninitialised value was created by a stack allocation
==2870==    at 0x459B1A: query_pin_retries_cb (sim.c:531)
==2870==
==2870== Conditional jump or move depends on uninitialised value(s)
==2870==    at 0x4F3DFB: get_pin_retries (sim.c:278)
==2870==    by 0x4F4553: sim_pin_retries_query_cb (sim.c:467)
==2870==    by 0x459BDD: query_pin_retries_cb (sim.c:544)
==2870==    by 0x45544A: service_send_callback (qmi.c:2143)
==2870==    by 0x452D00: handle_packet (qmi.c:815)
==2870==    by 0x452E85: received_data (qmi.c:863)
==2870==    by 0x508DB6C: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508DF47: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508E271: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x4C680B: main (main.c:256)
==2870==  Uninitialised value was created by a stack allocation
==2870==    at 0x459B1A: query_pin_retries_cb (sim.c:531)
==2870==
==2870== Conditional jump or move depends on uninitialised value(s)
==2870==    at 0x4F3E65: get_pin_retries (sim.c:288)
==2870==    by 0x4F4553: sim_pin_retries_query_cb (sim.c:467)
==2870==    by 0x459BDD: query_pin_retries_cb (sim.c:544)
==2870==    by 0x45544A: service_send_callback (qmi.c:2143)
==2870==    by 0x452D00: handle_packet (qmi.c:815)
==2870==    by 0x452E85: received_data (qmi.c:863)
==2870==    by 0x508DB6C: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508DF47: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x508E271: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==2870==    by 0x4C680B: main (main.c:256)
==2870==  Uninitialised value was created by a stack allocation
==2870==    at 0x459B1A: query_pin_retries_cb (sim.c:531)
Some gemalto modem family share almost the same base except some very
specific AT command and some hardware changes like ALSx and PXSx family
After setting up the request structure, qmi_service_send makes no
further use of the 'param' and 'service' fields of the service_send_data
structure.  This patch removes those fields and frees 'param'
immediately after the request has been allocated and the parameter data
thereby copied into the send buffer.
The headroom can be established from the service type, so it's redundant
to pass it as a parameter.
The service and control requests differ slightly in their headers, but
this difference is minor enough that we can handle it directly in the
request submission routine.  This patch unifies the header setup for the
two request types.
The only way request_alloc can fail is if one of the memory allocation
routines fail to allocate memory.  However, Linux memory allocation
doesn't really fail in this manner; memory can be overcommited and the
out-of-memory reaper will take care of re-establishing the balance when
excess memory is actually accessed.

Given this, request_alloc will never return anything other than success
and the failure paths will never be exercised.
The only thing this output parameter is being used for now is for
getting the transaction ID.  Return the TID directly from
__submit_requesta and drop the 'head' parameter altogether.
The way things are currently coded, the gobi plugin calls
qmi_device_discover and does nothing else until it succeeds.  As such,
we can safely assume that the version_list is set up when we go to
create a service.
Use right slot and application to get card status, PIN status and PIN
retries. Without this patch, SIMs where selected application and slot
numbers are different are not detected.
QMI_UIM_GET_CARD_STATUS is retried in more error cases
when trying to get password type.
In case of failure, driver report an error instead of
OFONO_SIM_PASSWORD_INVALID. This avoids a crash.
denkenz and others added 9 commits September 7, 2021 21:36
It seems that the function at_pin_send_puk should have been changed
along with at_pin_send, because it's also refering to the
at_pin_send_cb callback

See this commit : ba9f126
In case we try to enter the PIN/PUK and fail to enter a correct code,
the PIN/PUK retries are not rechecked as they should be.

Reported by: Florent Beillonnet <[email protected]>
gatchat/gatmux.c: In function ‘watch_dispatch’:
gatchat/gatmux.c:454:17: error: cast between incompatible function types from ‘GSourceFunc’ {aka ‘int (*)(void *)’} to ‘gboolean (*)(GIOChannel *, GIOCondition,  void *)’ {aka ‘int (*)(struct _GIOChannel *, enum <anonymous>,  void *)’} [-Werror=cast-function-type]
  GIOFunc func = (GIOFunc) callback;
                 ^
In file included from drivers/rilmodem/network-registration.c:40:
drivers/rilmodem/network-registration.c: In function ‘ril_cops_list_cb’:
./gril/gril.h:98:11: error: passing argument 1 to restrict-qualified parameter aliases with argument 3 [-Werror=restrict]
   sprintf(print_buf, x);  \
           ^~~~~~~~~
drivers/rilmodem/network-registration.c:583:3: note: in expansion of macro ‘g_ril_append_print_buf’
   g_ril_append_print_buf(nd->ril, "%s [lalpha=%s, salpha=%s, "
   ^~~~~~~~~~~~~~~~~~~~~~
./gril/gril.h:98:11: error: passing argument 1 to restrict-qualified parameter aliases with argument 3 [-Werror=restrict]
   sprintf(print_buf, x);  \
           ^~~~~~~~~
drivers/rilmodem/network-registration.c:593:2: note: in expansion of macro ‘g_ril_append_print_buf’
  g_ril_append_print_buf(nd->ril, "%s}", print_buf);
  ^~~~~~~~~~~~~~~~~~~~~~

In file included from drivers/rilmodem/call-forwarding.c:41:
drivers/rilmodem/call-forwarding.c: In function ‘ril_query_call_fwd_cb’:
./gril/gril.h:98:11: error: passing argument 1 to restrict-qualified parameter aliases with argument 3 [-Werror=restrict]
   sprintf(print_buf, x);  \
           ^~~~~~~~~
drivers/rilmodem/call-forwarding.c:114:3: note: in expansion of macro ‘g_ril_append_print_buf’
   g_ril_append_print_buf(fd->ril, "%s [%d,%d,%d,%s,%d]",
   ^~~~~~~~~~~~~~~~~~~~~~
./gril/gril.h:98:11: error: passing argument 1 to restrict-qualified parameter aliases with argument 3 [-Werror=restrict]
   sprintf(print_buf, x);  \
           ^~~~~~~~~
drivers/rilmodem/call-forwarding.c:124:2: note: in expansion of macro ‘g_ril_append_print_buf’
  g_ril_append_print_buf(fd->ril, "%s}", print_buf);
  ^~~~~~~~~~~~~~~~~~~~~~
gatchat/gatmux.c:33:32: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
 #pragma GCC diagnostic ignored "-Wcast-function-type"
                                ^~~~~~~~~~~~~~~~~~~~~~
@@ -38,7 +38,7 @@ typedef void (*ofono_lte_cb_t)(const struct ofono_error *error, void *data);

struct ofono_lte_driver {
const char *name;
int (*probe)(struct ofono_lte *lte, void *data);
int (*probe)(struct ofono_lte *lte, unsigned int vendor, void *data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that since it breaks plugin API. Upstream plugin API is considered unstable - we can't afford that.
On the other hand I'm not aware of any external plugin actually using this API...
IIRC this was one of the reasons why I stopped at 1.23

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay. I wasn't aware of that to be honest. I'm completely new to Sailfish actually. My only experience with Ofono before is in the use of Plasma Mobile on the PinePhone. Over there we use version 1.31, which for the PinePhone, which at the moment is quite stable. Of course the PinePhone isn't really comparable with most other devices of course, and Plasma Mobile also doesn't use the SFOS specific stuff.

@monich
Copy link
Member

monich commented Sep 7, 2021

Stepping back a bit and looking at it from a grand scale - is there anything really important in those commits, worth accepting the lte plugin API break?

@PsychoGame
Copy link
Contributor Author

This API break, is this purely related to these lte API changes? In that case I can revert these patches.
The patches which are most important for PinePhone are the QMI/Gobi related ones.
Currently PinePhone uses a separate fork of the SFOS Ofono tree, to be able to get the necessary modem functions up-and-running.

@monich
Copy link
Member

monich commented Sep 8, 2021

If there are important changes there then I'd say there's no need to work around the API break since there are no known external plugins which actually use that API. That looks like a reasonable safety vs complexity trade-off to me.

If my #4 can be easily rebased on top of your changes, I would even consider merging your stuff first and then mine on top of it. I'll have to test that first, of course.

@PsychoGame
Copy link
Contributor Author

Thank you for your willingness to think with me, and for sharing your opinion on this matter. Like I said, to me the most important changes are related to QMI/Gobi exclusively. But while I was busy with cherry-picking these commits, I thought of cherry-picking the rest as well to update Ofono. I wasn't aware that some of it would break the LTE API. If I were to cherry-pick more commits from the upstream branch, would you like me to be more specific in cherry-picking only commits which are neccessary for my case? Or is it preferred to get the complete Ofono stack more up-to-date (without breaking anything more of course)?
At least I share your opinion that it is preferrable to use the SFOS Ofono package for PinePhone instead of creating yet another fork, as this would increase the workload in keeping everything up-to-date.
At least I'm eager to learn, and to contribute my free time to maintain this package together with you guys from Jolla
.I'll keep an eye on this pull request and await your answer.

@monich
Copy link
Member

monich commented Sep 13, 2021

I have tested it on a real device, haven't noticed any regressions or anything suspicious. I estimate the risk of LTE API break biting us in the butt as being quite low and not worth the trouble of working around it. Let's keep it as is.

Copy link
Member

@monich monich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the contribution!

@monich monich merged commit c45e207 into sailfishos:master Sep 13, 2021
@PsychoGame
Copy link
Contributor Author

No problem I'm happy I can help out in any way. I'll oontinue my work on 1.25, and do some testing on that version.

@monich
Copy link
Member

monich commented Sep 15, 2021

FYI, one thing that I absolutely don't want to see in this fork is the ell dependency

@PsychoGame
Copy link
Contributor Author

Alright. Thank you for the notification. I'll make sure that the ELL dependency will be left alone.

piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 13, 2022
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
    sailfishos#5 0x7fa0516d6f6f  (/usr/lib/libglib-2.0.so.0+0x69f6f)
    sailfishos#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
    sailfishos#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
    sailfishos#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
    sailfishos#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#10 0x7fa0516d727e in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a27e)
    sailfishos#11 0x7fa0516d91c0  (/usr/lib/libglib-2.0.so.0+0x6c1c0)
    sailfishos#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
    sailfishos#13 0x5566b6429b1b in main ../git/src/main.c:286
    sailfishos#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    sailfishos#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
    sailfishos#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
    sailfishos#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
    sailfishos#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#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
    sailfishos#5 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#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
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 13, 2022
With the reference in place in received_data(), the address sanitizer
now encounters a use-after-free when the destroy notification is
dispatched for the read watcher (see below).

Fix this by remove the destroy notification callback, as it isn't really
used except in the shutdown function.

==5797==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000ac5904 at pc 0x55c1243b1f14 bp 0x7ffdef001340 sp 0x7ffdef001330
WRITE of size 4 at 0x621000ac5904 thread T0
    #0 0x55c1243b1f13 in read_watcher_destroy_notify ../git/gatchat/gatmux.c:660
    #1 0x7f08a8676742  (/usr/lib/libglib-2.0.so.0+0x62742)
    #2 0x7f08a867e2e4 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2e4)
    #3 0x7f08a8680210  (/usr/lib/libglib-2.0.so.0+0x6c210)
    #4 0x7f08a8681122 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d122)
    sailfishos#5 0x55c1243d6703 in main ../git/src/main.c:286
    sailfishos#6 0x7f08a8423152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    sailfishos#7 0x55c1241fe1ad in _start (/home/martin/projects/ofono/x86/src/ofonod+0xfd1ad)

0x621000ac5904 is located 4 bytes inside of 4672-byte region [0x621000ac5900,0x621000ac6b40)
freed by thread T0 here:
    #0 0x7f08a88cc6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x55c1243b1ebf in g_at_mux_unref ../git/gatchat/gatmux.c:652
    #2 0x55c1243b062c in received_data ../git/gatchat/gatmux.c:276
    #3 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

previously allocated by thread T0 here:
    #0 0x7f08a88cccd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x55c1243b1bf1 in g_at_mux_new ../git/gatchat/gatmux.c:613
    #2 0x55c1243b4b53 in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1172
    #3 0x55c124386abd in cmux_gatmux ../git/plugins/quectel.c:871
    #4 0x55c12438779f in cmux_cb ../git/plugins/quectel.c:1023
    sailfishos#5 0x55c1243a368e in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x55c1243a3bc8 in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x55c1243a4408 in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x55c1243a539e in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x55c1243ae2f9 in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:660 in read_watcher_destroy_notify
Shadow bytes around the buggy address:
  0x0c4280150ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280150b20:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b70: 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
==5797==ABORTING
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
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
    sailfishos#5 0x7fa0516d6f6f  (/usr/lib/libglib-2.0.so.0+0x69f6f)
    sailfishos#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
    sailfishos#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
    sailfishos#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
    sailfishos#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#10 0x7fa0516d727e in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a27e)
    sailfishos#11 0x7fa0516d91c0  (/usr/lib/libglib-2.0.so.0+0x6c1c0)
    sailfishos#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
    sailfishos#13 0x5566b6429b1b in main ../git/src/main.c:286
    sailfishos#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    sailfishos#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
    sailfishos#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
    sailfishos#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
    sailfishos#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#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
    sailfishos#5 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#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
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
With the reference in place in received_data(), the address sanitizer
now encounters a use-after-free when the destroy notification is
dispatched for the read watcher (see below).

Fix this by remove the destroy notification callback, as it isn't really
used except in the shutdown function.

==5797==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000ac5904 at pc 0x55c1243b1f14 bp 0x7ffdef001340 sp 0x7ffdef001330
WRITE of size 4 at 0x621000ac5904 thread T0
    #0 0x55c1243b1f13 in read_watcher_destroy_notify ../git/gatchat/gatmux.c:660
    #1 0x7f08a8676742  (/usr/lib/libglib-2.0.so.0+0x62742)
    #2 0x7f08a867e2e4 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2e4)
    #3 0x7f08a8680210  (/usr/lib/libglib-2.0.so.0+0x6c210)
    #4 0x7f08a8681122 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d122)
    sailfishos#5 0x55c1243d6703 in main ../git/src/main.c:286
    sailfishos#6 0x7f08a8423152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    sailfishos#7 0x55c1241fe1ad in _start (/home/martin/projects/ofono/x86/src/ofonod+0xfd1ad)

0x621000ac5904 is located 4 bytes inside of 4672-byte region [0x621000ac5900,0x621000ac6b40)
freed by thread T0 here:
    #0 0x7f08a88cc6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x55c1243b1ebf in g_at_mux_unref ../git/gatchat/gatmux.c:652
    #2 0x55c1243b062c in received_data ../git/gatchat/gatmux.c:276
    #3 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

previously allocated by thread T0 here:
    #0 0x7f08a88cccd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x55c1243b1bf1 in g_at_mux_new ../git/gatchat/gatmux.c:613
    #2 0x55c1243b4b53 in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1172
    #3 0x55c124386abd in cmux_gatmux ../git/plugins/quectel.c:871
    #4 0x55c12438779f in cmux_cb ../git/plugins/quectel.c:1023
    sailfishos#5 0x55c1243a368e in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x55c1243a3bc8 in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x55c1243a4408 in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x55c1243a539e in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x55c1243ae2f9 in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:660 in read_watcher_destroy_notify
Shadow bytes around the buggy address:
  0x0c4280150ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280150b20:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b70: 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
==5797==ABORTING
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
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
    sailfishos#5 0x7fa0516d6f6f  (/usr/lib/libglib-2.0.so.0+0x69f6f)
    sailfishos#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
    sailfishos#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
    sailfishos#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
    sailfishos#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#10 0x7fa0516d727e in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a27e)
    sailfishos#11 0x7fa0516d91c0  (/usr/lib/libglib-2.0.so.0+0x6c1c0)
    sailfishos#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
    sailfishos#13 0x5566b6429b1b in main ../git/src/main.c:286
    sailfishos#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    sailfishos#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
    sailfishos#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
    sailfishos#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
    sailfishos#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#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
    sailfishos#5 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#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
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
With the reference in place in received_data(), the address sanitizer
now encounters a use-after-free when the destroy notification is
dispatched for the read watcher (see below).

Fix this by remove the destroy notification callback, as it isn't really
used except in the shutdown function.

==5797==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000ac5904 at pc 0x55c1243b1f14 bp 0x7ffdef001340 sp 0x7ffdef001330
WRITE of size 4 at 0x621000ac5904 thread T0
    #0 0x55c1243b1f13 in read_watcher_destroy_notify ../git/gatchat/gatmux.c:660
    #1 0x7f08a8676742  (/usr/lib/libglib-2.0.so.0+0x62742)
    #2 0x7f08a867e2e4 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2e4)
    #3 0x7f08a8680210  (/usr/lib/libglib-2.0.so.0+0x6c210)
    #4 0x7f08a8681122 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d122)
    sailfishos#5 0x55c1243d6703 in main ../git/src/main.c:286
    sailfishos#6 0x7f08a8423152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    sailfishos#7 0x55c1241fe1ad in _start (/home/martin/projects/ofono/x86/src/ofonod+0xfd1ad)

0x621000ac5904 is located 4 bytes inside of 4672-byte region [0x621000ac5900,0x621000ac6b40)
freed by thread T0 here:
    #0 0x7f08a88cc6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x55c1243b1ebf in g_at_mux_unref ../git/gatchat/gatmux.c:652
    #2 0x55c1243b062c in received_data ../git/gatchat/gatmux.c:276
    #3 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

previously allocated by thread T0 here:
    #0 0x7f08a88cccd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x55c1243b1bf1 in g_at_mux_new ../git/gatchat/gatmux.c:613
    #2 0x55c1243b4b53 in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1172
    #3 0x55c124386abd in cmux_gatmux ../git/plugins/quectel.c:871
    #4 0x55c12438779f in cmux_cb ../git/plugins/quectel.c:1023
    sailfishos#5 0x55c1243a368e in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x55c1243a3bc8 in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x55c1243a4408 in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x55c1243a539e in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x55c1243ae2f9 in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:660 in read_watcher_destroy_notify
Shadow bytes around the buggy address:
  0x0c4280150ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280150b20:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b70: 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
==5797==ABORTING
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
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
    sailfishos#5 0x7fa0516d6f6f  (/usr/lib/libglib-2.0.so.0+0x69f6f)
    sailfishos#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
    sailfishos#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
    sailfishos#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
    sailfishos#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#10 0x7fa0516d727e in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a27e)
    sailfishos#11 0x7fa0516d91c0  (/usr/lib/libglib-2.0.so.0+0x6c1c0)
    sailfishos#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
    sailfishos#13 0x5566b6429b1b in main ../git/src/main.c:286
    sailfishos#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    sailfishos#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
    sailfishos#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
    sailfishos#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
    sailfishos#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#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
    sailfishos#5 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#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
piggz pushed a commit to sailfish-on-dontbeevil/ofono-new that referenced this pull request Feb 16, 2022
With the reference in place in received_data(), the address sanitizer
now encounters a use-after-free when the destroy notification is
dispatched for the read watcher (see below).

Fix this by remove the destroy notification callback, as it isn't really
used except in the shutdown function.

==5797==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000ac5904 at pc 0x55c1243b1f14 bp 0x7ffdef001340 sp 0x7ffdef001330
WRITE of size 4 at 0x621000ac5904 thread T0
    #0 0x55c1243b1f13 in read_watcher_destroy_notify ../git/gatchat/gatmux.c:660
    #1 0x7f08a8676742  (/usr/lib/libglib-2.0.so.0+0x62742)
    #2 0x7f08a867e2e4 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2e4)
    #3 0x7f08a8680210  (/usr/lib/libglib-2.0.so.0+0x6c210)
    #4 0x7f08a8681122 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d122)
    sailfishos#5 0x55c1243d6703 in main ../git/src/main.c:286
    sailfishos#6 0x7f08a8423152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    sailfishos#7 0x55c1241fe1ad in _start (/home/martin/projects/ofono/x86/src/ofonod+0xfd1ad)

0x621000ac5904 is located 4 bytes inside of 4672-byte region [0x621000ac5900,0x621000ac6b40)
freed by thread T0 here:
    #0 0x7f08a88cc6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x55c1243b1ebf in g_at_mux_unref ../git/gatchat/gatmux.c:652
    #2 0x55c1243b062c in received_data ../git/gatchat/gatmux.c:276
    #3 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

previously allocated by thread T0 here:
    #0 0x7f08a88cccd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x55c1243b1bf1 in g_at_mux_new ../git/gatchat/gatmux.c:613
    #2 0x55c1243b4b53 in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1172
    #3 0x55c124386abd in cmux_gatmux ../git/plugins/quectel.c:871
    #4 0x55c12438779f in cmux_cb ../git/plugins/quectel.c:1023
    sailfishos#5 0x55c1243a368e in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x55c1243a3bc8 in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x55c1243a4408 in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x55c1243a539e in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x55c1243ae2f9 in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:660 in read_watcher_destroy_notify
Shadow bytes around the buggy address:
  0x0c4280150ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280150b20:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b70: 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
==5797==ABORTING
neochapay pushed a commit to neochapay/ofono-new that referenced this pull request Dec 9, 2022
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
    sailfish-on-dontbeevil#2 0x7fa0516e44a6 in g_io_channel_shutdown (/usr/lib/libglib-2.0.so.0+0x774a6)
    sailfish-on-dontbeevil#3 0x7fa0516e4644 in g_io_channel_unref (/usr/lib/libglib-2.0.so.0+0x77644)
    sailfish-on-dontbeevil#4 0x5566b64048a4 in watch_finalize ../git/gatchat/gatmux.c:474
    sailfishos#5 0x7fa0516d6f6f  (/usr/lib/libglib-2.0.so.0+0x69f6f)
    sailfishos#6 0x7fa0516ac6a7 in g_slist_foreach (/usr/lib/libglib-2.0.so.0+0x3f6a7)
    sailfishos#7 0x7fa0516b277b in g_slist_free_full (/usr/lib/libglib-2.0.so.0+0x4577b)
    sailfishos#8 0x5566b6403413 in dispatch_sources ../git/gatchat/gatmux.c:224
    sailfishos#9 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#10 0x7fa0516d727e in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a27e)
    sailfishos#11 0x7fa0516d91c0  (/usr/lib/libglib-2.0.so.0+0x6c1c0)
    sailfishos#12 0x7fa0516da0d2 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d0d2)
    sailfishos#13 0x5566b6429b1b in main ../git/src/main.c:286
    sailfishos#14 0x7fa05147fee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)
    sailfishos#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
    sailfish-on-dontbeevil#2 0x5566b63d6d19 in close_mux ../git/plugins/quectel.c:199
    sailfish-on-dontbeevil#3 0x5566b63d7047 in close_serial ../git/plugins/quectel.c:223
    sailfish-on-dontbeevil#4 0x5566b63db62a in cfun_disable ../git/plugins/quectel.c:1056
    sailfishos#5 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x5566b64047b4 in watch_dispatch ../git/gatchat/gatmux.c:464
    sailfishos#11 0x5566b640313b in dispatch_sources ../git/gatchat/gatmux.c:183
    sailfishos#12 0x5566b64039ea in received_data ../git/gatchat/gatmux.c:268
    sailfishos#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
    sailfish-on-dontbeevil#2 0x5566b6407f6b in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1165
    sailfish-on-dontbeevil#3 0x5566b63da9ba in cmux_cb ../git/plugins/quectel.c:882
    sailfish-on-dontbeevil#4 0x5566b63f6ae1 in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#5 0x5566b63f701b in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#6 0x5566b63f785b in have_line ../git/gatchat/gatchat.c:600
    sailfishos#7 0x5566b63f87f1 in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#8 0x5566b640174c in received_data ../git/gatchat/gatio.c:122
    sailfishos#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
neochapay pushed a commit to neochapay/ofono-new that referenced this pull request Dec 9, 2022
With the reference in place in received_data(), the address sanitizer
now encounters a use-after-free when the destroy notification is
dispatched for the read watcher (see below).

Fix this by remove the destroy notification callback, as it isn't really
used except in the shutdown function.

==5797==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000ac5904 at pc 0x55c1243b1f14 bp 0x7ffdef001340 sp 0x7ffdef001330
WRITE of size 4 at 0x621000ac5904 thread T0
    #0 0x55c1243b1f13 in read_watcher_destroy_notify ../git/gatchat/gatmux.c:660
    #1 0x7f08a8676742  (/usr/lib/libglib-2.0.so.0+0x62742)
    sailfish-on-dontbeevil#2 0x7f08a867e2e4 in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2e4)
    sailfish-on-dontbeevil#3 0x7f08a8680210  (/usr/lib/libglib-2.0.so.0+0x6c210)
    sailfish-on-dontbeevil#4 0x7f08a8681122 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x6d122)
    sailfishos#5 0x55c1243d6703 in main ../git/src/main.c:286
    sailfishos#6 0x7f08a8423152 in __libc_start_main (/usr/lib/libc.so.6+0x27152)
    sailfishos#7 0x55c1241fe1ad in _start (/home/martin/projects/ofono/x86/src/ofonod+0xfd1ad)

0x621000ac5904 is located 4 bytes inside of 4672-byte region [0x621000ac5900,0x621000ac6b40)
freed by thread T0 here:
    #0 0x7f08a88cc6b0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x55c1243b1ebf in g_at_mux_unref ../git/gatchat/gatmux.c:652
    sailfish-on-dontbeevil#2 0x55c1243b062c in received_data ../git/gatchat/gatmux.c:276
    sailfish-on-dontbeevil#3 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

previously allocated by thread T0 here:
    #0 0x7f08a88cccd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
    #1 0x55c1243b1bf1 in g_at_mux_new ../git/gatchat/gatmux.c:613
    sailfish-on-dontbeevil#2 0x55c1243b4b53 in g_at_mux_new_gsm0710_basic ../git/gatchat/gatmux.c:1172
    sailfish-on-dontbeevil#3 0x55c124386abd in cmux_gatmux ../git/plugins/quectel.c:871
    sailfish-on-dontbeevil#4 0x55c12438779f in cmux_cb ../git/plugins/quectel.c:1023
    sailfishos#5 0x55c1243a368e in at_chat_finish_command ../git/gatchat/gatchat.c:459
    sailfishos#6 0x55c1243a3bc8 in at_chat_handle_command_response ../git/gatchat/gatchat.c:521
    sailfishos#7 0x55c1243a4408 in have_line ../git/gatchat/gatchat.c:600
    sailfishos#8 0x55c1243a539e in new_bytes ../git/gatchat/gatchat.c:759
    sailfishos#9 0x55c1243ae2f9 in received_data ../git/gatchat/gatio.c:122
    sailfishos#10 0x7f08a867e2ce in g_main_context_dispatch (/usr/lib/libglib-2.0.so.0+0x6a2ce)

SUMMARY: AddressSanitizer: heap-use-after-free ../git/gatchat/gatmux.c:660 in read_watcher_destroy_notify
Shadow bytes around the buggy address:
  0x0c4280150ad0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150ae0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150af0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280150b10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c4280150b20:[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c4280150b70: 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
==5797==ABORTING
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants