bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

[Ken Brown]

The following elisp file, extracted from dbus-tests.el, causes a crash on 32-bit Cygwin unless emacs is built --with-wide-int.

$ cat dbus-crash.el
(require 'dbus)
(setq output
      (shell-command-to-string "dbus-launch --sh-syntax"))
(when (string-match "DBUS_SESSION_BUS_ADDRESS='\\(.+\\)';" output)
  (setq bus (match-string 1 output)))
(dbus-init-bus bus)

$ emacs -Q -l dbus-crash.el
Fatal error 11: Segmentation faultSegmentation fault (core dumped)

If there is already a session bus running before starting emacs, simply loading the dbus library is enough to elicit the crash.

The problem is this line in Fdbus__init_bus in dbusbind.c:

      XSETFASTINT (val, (intptr_t) connection);

Here 'connection' is a 32-bit pointer, which may be too big to be treated as an integer in Emacs.  In principle this could happen on any 32-bit platform, but it is more likely to happen on Cygwin because the heap can be in high memory.  The following gdb session illustrates this.

(Note: For simplicity, I started a session bus before starting emacs, so that I only had to load dbus to get a crash.)

$ gdb emacs
GNU gdb (GDB) (Cygwin 7.10.1-1) 7.10.1
[...]
Breakpoint 1 at 0x511474: file ../../emacs-25/src/emacs.c, line 354.
Temporary breakpoint 2 at 0x530bc1: file ../../emacs-25/src/sysdep.c, line 915.
(gdb) b Fdbus__init_bus
Breakpoint 3 at 0x50b41d: file ../../emacs-25/src/dbusbind.c, line 1124.
(gdb) r -Q
Starting program: /home/kbrown/src/emacs/32build-emacs-25/src/emacs -Q

[At this point I did M-x load-library RET dbus RET .]

Breakpoint 3, Fdbus__init_bus (bus=816, private=0)
    at ../../emacs-25/src/dbusbind.c:1124
1124      XD_DBUS_VALIDATE_BUS_ADDRESS (bus);
(gdb) p bus
$1 = 816
(gdb) pr
:system
(gdb) c
Continuing.

Breakpoint 3, Fdbus__init_bus (bus=792, private=0)
    at ../../emacs-25/src/dbusbind.c:1124
1124      XD_DBUS_VALIDATE_BUS_ADDRESS (bus);
(gdb) p bus
$2 = 792
(gdb) pr
:session
(gdb) n
[...]
1151              connection = dbus_bus_get (EQ (bus, QCdbus_system_bus)
(gdb) 
1159          if (dbus_error_is_set (&derror))
(gdb) p connection
$3 = (DBusConnection *) 0x2007a960
(gdb) n
[...]
1190          XSETFASTINT (val, (intptr_t) connection);
(gdb) 
1191          xd_registered_buses = Fcons (Fcons (bus, val), xd_registered_buses);
(gdb) p/x val
$5 = 0x801ea582
(gdb) pr
-536368800

[At this point xd_registered_buses contains a bogus connection address for the session bus.]

(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x67e2910d in dbus_connection_get_is_connected (connection=0xe007a960)
    at /usr/src/debug/dbus-1.10.8-2/dbus/dbus-connection.c:2979

The bogus value 0xe007a960 (instead of 0x2007a960) was retrieved from xd_registered_buses and caused the crash.

I suspect that there is a simple solution, which involves storing the connection address in a Lisp Object of type other than integer, but I'll leave that to the experts.

In GNU Emacs 25.0.94.1 (i686-pc-cygwin, GTK+ Version 3.18.9)
 of 2016-06-10 built on desktop-new
Repository revision: 66d556b5187d768bbd233513b54dcb4beaa90c6d
Windowing system distributor 'The Cygwin/X Project', version 11.0.11802000
Configured using:
 'configure 'CFLAGS=-g -O0''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GCONF GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

[Ken Brown]

On 6/10/2016 12:12 PM, Ken Brown wrote:
> I suspect that there is a simple solution, which involves storing the connection address in a Lisp Object of type other than integer, but I'll leave that to the experts.

The following seems to fix it:

diff --git a/src/dbusbind.c b/src/dbusbind.c
index d3a32c0..56bfd71 100644
--- a/src/dbusbind.c
+++ b/src/dbusbind.c
@@ -943,7 +943,7 @@ xd_get_connection_references (DBusConnection *connection)
 static DBusConnection*
 xd_lisp_dbus_to_dbus (Lisp_Object bus)
 {
-  return (DBusConnection *) (intptr_t) XFASTINT (bus);
+  return (DBusConnection *) XSAVE_POINTER (bus, 0);
 }

 /* Return D-Bus connection address.  BUS is either a Lisp symbol,
@@ -1187,7 +1187,7 @@ this connection to those buses.  */)
        XD_SIGNAL1 (build_string ("Cannot add watch functions"));

       /* Add bus to list of registered buses.  */
-      XSETFASTINT (val, (intptr_t) connection);
+      val = make_save_ptr (connection);
       xd_registered_buses = Fcons (Fcons (bus, val), xd_registered_buses);

       /* Cleanup.  */


Ken

bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

[Eli Zaretskii]

> From: Ken Brown <kbrown@cornell.edu>
> Date: Fri, 10 Jun 2016 17:11:11 -0400
> 
> On 6/10/2016 12:12 PM, Ken Brown wrote:
> > I suspect that there is a simple solution, which involves storing the connection address in a Lisp Object of type other than integer, but I'll leave that to the experts.
> 
> The following seems to fix it:
> 
> diff --git a/src/dbusbind.c b/src/dbusbind.c
> index d3a32c0..56bfd71 100644
> --- a/src/dbusbind.c
> +++ b/src/dbusbind.c
> @@ -943,7 +943,7 @@ xd_get_connection_references (DBusConnection *connection)
>  static DBusConnection*
>  xd_lisp_dbus_to_dbus (Lisp_Object bus)
>  {
> -  return (DBusConnection *) (intptr_t) XFASTINT (bus);
> +  return (DBusConnection *) XSAVE_POINTER (bus, 0);
>  }
> 
>  /* Return D-Bus connection address.  BUS is either a Lisp symbol,
> @@ -1187,7 +1187,7 @@ this connection to those buses.  */)
>         XD_SIGNAL1 (build_string ("Cannot add watch functions"));
> 
>        /* Add bus to list of registered buses.  */
> -      XSETFASTINT (val, (intptr_t) connection);
> +      val = make_save_ptr (connection);
>        xd_registered_buses = Fcons (Fcons (bus, val), xd_registered_buses);
> 
>        /* Cleanup.  */

Thanks, please push to the release branch.

bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

[Michael Albinus]

Ken Brown <kbrown@cornell.edu> writes:

> On 6/10/2016 12:12 PM, Ken Brown wrote:
>> I suspect that there is a simple solution, which involves storing
>> the connection address in a Lisp Object of type other than integer,
>> but I'll leave that to the experts.
>
> The following seems to fix it:

Looks good to me. dbus-tests.el passes successfully on Ubuntu 16.04
(64bit) and Ubuntu 12.04 (32bit).

> Ken

Best regards, Michael.

bug#23741: 25.0.94; dbus crashes on 32-bit Cygwin

[Ken Brown]

On 6/11/2016 3:34 AM, Eli Zaretskii wrote:
> Thanks, please push to the release branch.

Done, as commit 6921f4a.  Closing.