After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 459323 - Crash when calling getAddress on a Network object with dbus-send
Crash when calling getAddress on a Network object with dbus-send
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
0.6.6
Other Linux
: Normal normal
: ---
Assigned To: Dan Williams
Dan Williams
Depends on:
Blocks:
 
 
Reported: 2007-07-22 20:12 UTC by Ross Vandegrift
Modified: 2008-08-16 03:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Allocate buf from the heap since Dbus needs to access it while marshalling a string (446 bytes, patch)
2008-05-11 20:39 UTC, Ross Vandegrift
none Details | Review

Description Ross Vandegrift 2007-07-22 20:12:52 UTC
I've been exploring talking to NetworkManager, and have found a crash with NetworkManager 0.6.4 from Ubuntu Feisty Fawn.  All I need to to do to crash NetworkManager is run this command:

dbus-send --system --print-reply --type=method_call \ 
--dest=org.freedesktop.NetworkManager \ 
/org/freedesktop/NetworkManager/Devices/eth1/Networks/eliza1 \
org.freedesktop.NetworkManager.Devices.getAddress

where ".../eth1" is the wireless device in my laptop and ".../eliza1" is the wireless network I'm connected to.  The network is encrypted with WEP.

NetworkManagerDispatcher remains running after this.  Other calls with this same command (for example "...getName") behave as expected.
Comment 1 Dan Williams 2007-12-07 15:09:58 UTC
Any chance you could install debuginfo/symbols for NetworkManager, gdb attach to it before calling this function, and provide a backtrace?
Comment 2 Ross Vandegrift 2007-12-15 20:24:19 UTC
(In reply to comment #1)
> Any chance you could install debuginfo/symbols for NetworkManager, gdb attach
> to it before calling this function, and provide a backtrace?

Sure.  I've since upgraded to Gutsy Gibbon, which comes with NetworkManager 0.6.5, but I can still trigger the  bug.

I've got some debugging symbols installed for glib and rebuild network-manager with debugging symbols.  Here's the results of "thread apply all backtrace" after triggering the crash:

(gdb) thread apply all bt


Comment 3 Ross Vandegrift 2007-12-15 20:36:56 UTC
(In reply to comment #1)
> Any chance you could install debuginfo/symbols for NetworkManager, gdb attach
> to it before calling this function, and provide a backtrace?

One more note - I saw some bugs reported with 128-bit WEP networks in bugzilla, so I just reproduced this with an open wireless AP and see the same backtrace. 

Comment 4 Dan Williams 2008-04-27 21:51:24 UTC
Does this happen with 0.6.6 or higher?  the backtrace doesn't seem to show the crash, does gdb interrupt the program with a segfault warning?
Comment 5 Ross Vandegrift 2008-05-11 17:12:53 UTC
Yep, I still see this crash with 0.6.6 from Hardy Heron.  Here is a new thread apply all bt:

(gdb) cont
Continuing.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb7b9f720 (LWP 5373)]
0xb7ce6283 in strlen () from /lib/tls/i686/cmov/libc.so.6
(gdb) thread apply all bt

Thread 1 (Thread 0xb7b9f720 (LWP 5373))

  • #0 strlen
    from /lib/tls/i686/cmov/libc.so.6
  • #1 _dbus_marshal_write_basic
    at dbus-marshal-basic.c line 757
  • #2 _dbus_type_writer_write_basic_no_typecode
    at dbus-marshal-recursive.c line 1588
  • #3 _dbus_type_writer_write_basic
    at dbus-marshal-recursive.c line 2310
  • #4 dbus_message_iter_append_basic
    at dbus-message.c line 2247
  • #5 dbus_message_append_args_valist
    at dbus-message.c line 1534
  • #6 dbus_message_append_args
    at dbus-message.c line 1492
  • #7 nm_dbus_net_get_address
    at nm-dbus-net.c line 136
  • #8 nm_dbus_method_dispatch
    at NetworkManagerDbusUtils.c line 81
  • #9 nm_dbus_devices_message_handler
    at NetworkManagerDbus.c line 676
  • #10 _dbus_object_tree_dispatch_and_unlock
    at dbus-object-tree.c line 856
  • #11 dbus_connection_dispatch
    at dbus-connection.c line 4420
  • #12 message_queue_dispatch
    at dbus-gmain.c line 101
  • #13 IA__g_main_context_dispatch
    at /build/buildd/glib2.0-2.16.3/glib/gmain.c line 2009
  • #14 g_main_context_iterate
    at /build/buildd/glib2.0-2.16.3/glib/gmain.c line 2642
  • #15 IA__g_main_loop_run
    at /build/buildd/glib2.0-2.16.3/glib/gmain.c line 2850
  • #16 main
    at NetworkManager.c line 1060



I'm going to poke around on this crash a bit more and may try current development head.
Comment 6 Ross Vandegrift 2008-05-11 20:39:03 UTC
Created attachment 110730 [details] [review]
Allocate buf from the heap since Dbus needs to access it while marshalling a string
Comment 7 Ross Vandegrift 2008-05-11 20:40:14 UTC
I have found the problem.

In nm-dbus-net.c:nm_dbus_net_get_address(), NM does the following sequence:

char buf[20];
...
dbus_message_append_args (reply, DBUS_TYPE_STRING, &buf, DBUS_TYPE_INVALID);


Following dbus_message_append_args() down the call stack is _dbus_marshal_write_basic, which assumes that the pointer it was passed is on the heap:

dbus_bool_t
_dbus_marshal_write_basic (DBusString *str,
                           int         insert_at,
                           int         type,
                           const void *value,
                           int         byte_order,
                           int        *pos_after)
{
  const DBusBasicValue *vp;
  ...
  vp = value;


During the marshal of the string, dbus does strlen(vp->str) -> SEGV, since vp points to a variable on the stack opf nm_dbus_net_get_address().

The attached patch fixes this crash for me, but this seems like the kind of thing that could be in other places as well.  Should dbus really dereference values passed to it for message encapsulation?
Comment 8 Dan Williams 2008-08-16 03:56:57 UTC
thanks, fixed in svn r3972