Re: [MM] [PATCH v2] serial-port: avoid opening a serial port that has been disposed

[Ben Chan <benchan chromium org>]

The crash happens to be a reuse of an already disposed MMSerialPort object. I'm now tracing the new/dispose paths.

Ben

    /* Don't read any input if the current command isn't done being sent yet */

    info = g_queue_peek_nth (priv->queue, 0);

   49f60:       6940            ldr     r0, [r0, #20]                         <- 0x151F60

   49f62:       f7c9 ebf6       blx     13750 <_init+0x1254>

==20047== Invalid read of size 4

==20047==    at 0x151F60: ??? (in /usr/sbin/ModemManager)

==20047==  Address 0x14 is not stack'd, malloc'd or (recently) free'd

==20047== 

==20047== 

==20047== Process terminating with default action of signal 11 (SIGSEGV)

==20047==  Access not within mapped region at address 0x14

==20047==    at 0x151F60: ??? (in /usr/sbin/ModemManager)

==20047==  If you believe this happened as a result of a stack

==20047==  overflow in your program's main thread (unlikely but

==20047==  possible), you can try to increase the size of the

==20047==  main thread stack using the --main-stacksize= flag.

==20047==  The main thread stack size used in this run was 8388608.

(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed

(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed

(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

(ModemManager:2858): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed

(ModemManager:2858): GLib-GObject-WARNING **: gsignal.c:2576: instance `0x78624028' has no handler with id `148'

(ModemManager:2858): GLib-GObject-WARNING **: invalid unclassed pointer in cast to `MMSerialPort'

(ModemManager:2858): GLib-GObject-CRITICAL **: g_type_instance_get_private: assertion `instance != NULL && instance->g_class != NULL' failed

On Thu, Nov 29, 2012 at 11:26 AM, Dan Williams <dcbw redhat com> wrote:

On Tue, 2012-11-27 at 13:19 -0800, Ben Chan wrote:
> Yes, it's related to the data_available (mm-serial-port.c:767) crash
> (crosbug.com/35391).  I'm running suspend/resume stress test with
> ModemManager under valgrind.

Any way to get MM running with --debug in those logs?  Also, in your
sources, does line 767 match up with:

    info = g_queue_peek_nth (priv->queue, 0);

or some other line?

One other thing to do, put an

mm_info ("(%s): disposing", mm_port_get_device (MM_PORT (self)));

into dispose() and lets see when it gets disposed and if anything tries
to open it during/after dispose.  Drop another of these into
mm_serial_port_new().

Dan

>
> Thanks,
> Ben
>
> On Tue, Nov 27, 2012 at 1:13 PM, Aleksander Morgado
> <aleksander lanedo com> wrote:
>         On 11/27/2012 09:39 PM, Ben Chan wrote:
>         > ---
>         >  src/mm-serial-port.c |   10 ++++++++++
>         >  1 files changed, 10 insertions(+), 0 deletions(-)
>
>
>
>         Don't think it's the correct approach.
>
>         I don't think we ever run g_object_run_dispose() ourselves for
>         port
>         objects, and that means that whenever we got the port object
>         disposed it
>         was because it was the last valid reference.
>
>         And that means that you won't be able to read priv->disposed
>         afterwards
>         as that would mean reading already freed memory.
>
>         There clearly is a missing reference around, or a timeout or
>         other
>         source which has the port as user_data and is not being
>         properly cleaned
>         up (e.g. removing the timeout or event source when the port is
>         disposed). Running it under valgrind should confirm this. Is
>         this
>         related to the data_available() crashes during suspend/resume?
>
>
>         >
>         > diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
>         > index 0a8820d..a33c745 100644
>         > --- a/src/mm-serial-port.c
>         > +++ b/src/mm-serial-port.c
>         > @@ -69,6 +69,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
>         >
>         >  typedef struct {
>         >      guint32 open_count;
>         > +    gboolean disposed;
>         >      gboolean forced_close;
>         >      int fd;
>         >      GHashTable *reply_cache;
>         > @@ -849,6 +850,12 @@ mm_serial_port_open (MMSerialPort
>         *self, GError **error)
>         >
>         >      device = mm_port_get_device (MM_PORT (self));
>         >
>         > +    /* If the MMSerialPort object has been disposed, just
>         return an error. */
>         > +    if (priv->disposed) {
>         > +        mm_info ("(%s) skipped opening serial port that has
>         been disposed", device);
>         > +        return FALSE;
>         > +    }
>         > +
>         >      if (priv->open_count) {
>         >          /* Already open */
>         >          goto success;
>         > @@ -1537,6 +1544,9 @@ dispose (GObject *object)
>         >  {
>         >      MMSerialPortPrivate *priv = MM_SERIAL_PORT_GET_PRIVATE
>         (object);
>         >
>         > +    /* Mark the MMSerialPort object as disposed to prevent
>         it from being re-opened. */
>         > +    priv->disposed = TRUE;
>         > +
>         >      if (priv->timeout_id) {
>         >          g_source_remove (priv->timeout_id);
>         >          priv->timeout_id = 0;
>         >
>
>
>         --
>
>         Aleksander
>
>

> _______________________________________________
> networkmanager-list mailing list
> networkmanager-list gnome org
> https://mail.gnome.org/mailman/listinfo/networkmanager-list