Hi,
After around 70 passes overnight, I didn't stumble upon a segfault, so for me,
the patch seems solid. +1.
The test was deliberately exploiting a bug in modem causing it's primary AT
port to freeze and not respond to any commands, then waiting for MM to dispose
of the modem object, to trigger modem reboot.
Big thanks once again for very quick response!
With kind regards,
Lech
W dniu 22.01.2020 o 18:25, Aleksander Morgado pisze:
> Hey,
>
>>> While testing a mechanism to trigger a modem restart based on port timeout
>>> (and disposing of modem object in MM) I managed to segfault ModemManager.
>>> I'm using vanilla MM 1.12.4 built for armhf target.
>>>
>>> What I've found, is that a race condition is possible on
>>> serial_port_timed_out_cb, which is called one more time than it should,
>>> causing a use-after-free.
>>> I've found out that by the fact I used default value of max_timeouts (10)
>>> and the method got called with n_consecutive_timeouts = 11, but according
>>> to the condition there, the cancellable was triggered on previous call with
>>> n_consecutive_timeouts = 10.
>>>
>>> The crash happened when calling g_dbus_object_get_object_path
>>> (G_DBUS_OBJECT (self) when evaluating arguments for the error log in
>>> serial_port_timed_out_cb :
>>>
>>> Stack trace:
>>> #0 g_dbus_object_get_object_path (object=0x68f1c0) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gio/gdbusobject.c:110
>>> #1 0x0005844c in serial_port_timed_out_cb (port=<optimized out>,
>>> n_consecutive_timeouts=11, self=0x68f1c0)
>>> at
>>> /usr/src/debug/modemmanager/1.12+gitrAUTOINC+cef03ce291-r6/git/src/mm-base-modem.c:130
>>> #2 0x7699ce48 in g_cclosure_marshal_VOID__FLAGSv (closure=<optimized out>,
>>> return_value=<optimized out>, instance=<optimized out>, args=...,
>>> marshal_data=0x0, n_params=1,
>>> param_types=0x670fa0) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gobject/gmarshal.c:1570
>>> #3 0x7699aaec in _g_closure_invoke_va (closure=closure@entry=0x66e548,
>>> return_value=return_value@entry=0x0, instance=0x60ea18,
>>> instance@entry=0x6ac628, args=args@entry=..., n_params=1,
>>> param_types=0x670fa0) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gobject/gclosure.c:867
>>> #4 0x769b5f44 in g_signal_emit_valist (instance=instance@entry=0x6ac628,
>>> signal_id=<optimized out>, detail=detail@entry=0, var_args=...,
>>> var_args@entry=...)
>>> at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gobject/gsignal.c:3294
>>> #5 0x769b6580 in g_signal_emit (instance=instance@entry=0x6ac628,
>>> signal_id=<optimized out>, detail=0) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gobject/gsignal.c:3441
>>> #6 0x000d225c in port_serial_timed_out (data=<optimized out>) at
>>> /usr/src/debug/modemmanager/1.12+gitrAUTOINC+cef03ce291-r6/git/src/mm-port-serial.c:797
>>> #7 0x768d4758 in g_timeout_dispatch (source=0x682280, callback=0xd21d8
>>> <port_serial_timed_out>, user_data=<optimized out>)
>>> at /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/glib/gmain.c:4577
>>> #8 0x768d3b1c in g_main_dispatch (context=0x614bf8) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/glib/gmain.c:3154
>>> #9 g_main_context_dispatch (context=context@entry=0x614bf8) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/glib/gmain.c:3769
>>> #10 0x768d3eb4 in g_main_context_iterate (context=0x614bf8,
>>> block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
>>> at /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/glib/gmain.c:3840
>>> #11 0x768d4210 in g_main_loop_run (loop=0x61ef10) at
>>> /usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/glib/gmain.c:4034
>>> #12 0x00044714 in main (argc=<optimized out>, argv=<optimized out>) at
>>> /usr/src/debug/modemmanager/1.12+gitrAUTOINC+cef03ce291-r6/git/src/main.c:206
>>>
>>> Frame #1:
>>> (gdb) info frame
>>> Stack level 0, frame at 0x7e9fca90:
>>> pc = 0x76ae816c in g_dbus_object_get_object_path
>>> (/usr/src/debug/glib-2.0/1_2.48.2-r0/glib-2.48.2/gio/gdbusobject.c:110);
>>> saved pc = 0x5844c
>>> called by frame at 0x7e9fcac0
>>> source language c.
>>> Arglist at 0x7e9fca88, args: object=0x68f1c0
>>> Locals at 0x7e9fca88, Previous frame's sp is 0x7e9fca90
>>> Saved registers:
>>> r4 at 0x7e9fca88, lr at 0x7e9fca8c
>>> object = 0x68f1c0
>>>
>>> (gdb) info args
>>> port = <optimized out>
>>> n_consecutive_timeouts = 11
>>>
>>> (gdb) print *self
>>> $2 = {parent = {parent_instance = {parent_instance = {g_type_instance =
>>> {g_class = 0x0}, ref_count = 0, qdata = 0x0}, priv = 0x68f1b0}, priv =
>>> 0x0}, priv = 0x68f148}
>>> (gdb) print *self->priv
>>> $4 = {connection = 0x0, cancellable = 0x0, invalid_if_cancelled = 61,
>>> device = 0x6b22f0 "XEb", drivers = 0x6abda0, plugin = 0x653fc0 "\230\275j",
>>> vendor_id = 7111, product_id = 33,
>>> hotplugged = 0, valid = 0, reprobe = 0, max_timeouts = 10, authp = 0x0,
>>> authp_cancellable = 0x0, ports = 0x0, primary = 0x0, secondary = 0x0, qcdm
>>> = 0x0, data = 0x0, gps_control = 0x0,
>>> gps = 0x0, audio = 0x0, enable_tasks = 0x0, disable_tasks = 0x0, qmi =
>>> 0x0, mbim = 0x0}
>>>
>>> I have a full session log and a core dump, so pretty much anything could be
>>> extracted if needed. So far it only reproduced once. I'm also analyzing
>>> myself how to fix it, but I'd be glad if you could take a look at this.
>>>
>> The error makes sense; the port object is living for longer than the
>> modem object, and so by the time the timed-out signal is emitted, the
>> modem object is already unrefed. The fix for this issue is to keep
>> track of all signal ids returned by g_signal_connect(port,
>> "timed-out",...) and make sure they are disconnected when the modem
>> object is unref-ed. Let me try to patch that.
>>
> I believe this should now be fixed in the following merge request:
> https://gitlab.freedesktop.org/mobile-broadband/ModemManager/merge_requests/248
>
> If you're able to reproduce the problem easily, please give it a try
> and let me know if it fixes it or not. I'll backport the relevant
> change to 1.12.x once if the change in master looks ok.
>
> --
> Aleksander
> https://aleksander.es
> ___
> This email originated from outside of Camlin Group. Do not click on links or
> open attachments unless you are confident they are secure. If in doubt,
> contact IT Support (itsupp...@camlintechnologies.com).
--
Pozdrawiam/With kind regards,
Lech Perczak
Sr. Software Engineer
Camlin Technologies Poland Limited Sp. z o.o.
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
