emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Emacs-bug-tracker] bug#8722: closed (dbusbind.c fixes for integer issue


From: GNU bug Tracking System
Subject: [Emacs-bug-tracker] bug#8722: closed (dbusbind.c fixes for integer issues, e.g., integer overflow)
Date: Fri, 27 May 2011 20:41:03 +0000

Your message dated Fri, 27 May 2011 13:40:10 -0700
with message-id <address@hidden>
and subject line fixes committed to trunk
has caused the GNU bug report #8722,
regarding dbusbind.c fixes for integer issues, e.g., integer overflow
to be marked as done.

(If you believe you have received this mail in error, please contact
address@hidden)


-- 
8722: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8722
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- Subject: dbusbind.c fixes for integer issues, e.g., integer overflow Date: Mon, 23 May 2011 22:32:19 -0700 User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Thunderbird/3.1.10
I found some integer overflow issues in dbusbind.c and came up
with the following patch which I'd like to install.  This doesn't
fix all the integer overflow problems in dbusbind.c, but it makes
some progress anyway.

=== modified file 'src/ChangeLog'
--- src/ChangeLog       2011-05-24 01:20:04 +0000
+++ src/ChangeLog       2011-05-24 05:16:14 +0000
@@ -1,5 +1,23 @@
 2011-05-24  Paul Eggert  <address@hidden>
 
+       * dbusbind.c: Serial number integer overflow fixes.
+       (CHECK_DBUS_SERIAL_GET_SERIAL): New macro.
+       (xd_invalid_serial): New static function.
+       (Fdbus_call_method_asynchronously, xd_read_message_1): Use a float
+       to hold a serial number that is too large for a fixnum.
+       (Fdbus_method_return_internal, Fdbus_method_error_internal):
+       Check for serial numbers out of range.  Decode any serial number
+       that was so large that it became a float.
+
+       * dbusbind.c: Use XFASTINT rather than XUINT, and check for nonneg.
+       (Fdbus_call_method, Fdbus_call_method_asynchronously):
+       Use XFASTINT rather than XUINT when numbers are nonnegative.
+       (xd_append_arg, Fdbus_method_return_internal):
+       (Fdbus_method_error_internal): Likewise.  Also, for unsigned
+       arguments, check that Lisp number is nonnegative, rather than
+       silently wrapping negative numbers around.
+       (xd_read_message_1): Don't assume dbus_uint32_t can fit in int.
+
        * data.c (arith_driver, Flsh): Avoid unnecessary casts to EMACS_UINT.
 
 2011-05-23  Paul Eggert  <address@hidden>

=== modified file 'src/dbusbind.c'
--- src/dbusbind.c      2011-05-06 22:12:31 +0000
+++ src/dbusbind.c      2011-05-24 05:16:14 +0000
@@ -242,6 +242,31 @@
 #define XD_NEXT_VALUE(object)                                          \
   ((XD_DBUS_TYPE_P (CAR_SAFE (object))) ? CDR_SAFE (object) : object)
 
+/* Check whether X is a valid dbus serial number.  If valid, set
+   SERIAL to its value.  Otherwise, signal an error. */
+#define CHECK_DBUS_SERIAL_GET_SERIAL(x, serial)                                
\
+  do                                                                   \
+    {                                                                  \
+      dbus_uint32_t DBUS_SERIAL_MAX = -1;                              \
+      if (NATNUMP (x) && XINT (x) <= DBUS_SERIAL_MAX)                  \
+       serial = XINT (x);                                              \
+      else if (MOST_POSITIVE_FIXNUM < DBUS_SERIAL_MAX                  \
+              && FLOATP (x)                                            \
+              && 0 <= XFLOAT_DATA (x)                                  \
+              && XFLOAT_DATA (x) <= DBUS_SERIAL_MAX)                   \
+       serial = XFLOAT_DATA (x);                                       \
+      else                                                             \
+       xd_invalid_serial (x);                                          \
+    }                                                                  \
+  while (0)
+
+static void xd_invalid_serial (Lisp_Object) NO_RETURN;
+static void
+xd_invalid_serial (Lisp_Object x)
+{
+  signal_error ("Invalid dbus serial", x);
+}
+
 /* Compute SIGNATURE of OBJECT.  It must have a form that it can be
    used in dbus_message_iter_open_container.  DTYPE is the DBusType
    the object is related to.  It is passed as argument, because it
@@ -431,9 +456,9 @@
     switch (dtype)
       {
       case DBUS_TYPE_BYTE:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         unsigned char val = XUINT (object) & 0xFF;
+         unsigned char val = XFASTINT (object) & 0xFF;
          XD_DEBUG_MESSAGE ("%c %d", dtype, val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -460,9 +485,9 @@
        }
 
       case DBUS_TYPE_UINT16:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint16_t val = XUINT (object);
+         dbus_uint16_t val = XFASTINT (object);
          XD_DEBUG_MESSAGE ("%c %u", dtype, (unsigned int) val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -483,9 +508,9 @@
 #ifdef DBUS_TYPE_UNIX_FD
       case DBUS_TYPE_UNIX_FD:
 #endif
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint32_t val = XUINT (object);
+         dbus_uint32_t val = XFASTINT (object);
          XD_DEBUG_MESSAGE ("%c %u", dtype, val);
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
@@ -503,10 +528,10 @@
        }
 
       case DBUS_TYPE_UINT64:
-       CHECK_NUMBER (object);
+       CHECK_NATNUM (object);
        {
-         dbus_uint64_t val = XUINT (object);
-         XD_DEBUG_MESSAGE ("%c %"pI"u", dtype, XUINT (object));
+         dbus_uint64_t val = XFASTINT (object);
+         XD_DEBUG_MESSAGE ("%c %"pI"d", dtype, XFASTINT (object));
          if (!dbus_message_iter_append_basic (iter, dtype, &val))
            XD_SIGNAL2 (build_string ("Unable to append argument"), object);
          return;
@@ -1110,7 +1135,7 @@
   if ((i+2 <= nargs) && (EQ ((args[i]), QCdbus_timeout)))
     {
       CHECK_NATNUM (args[i+1]);
-      timeout = XUINT (args[i+1]);
+      timeout = XFASTINT (args[i+1]);
       i = i+2;
     }
 
@@ -1186,7 +1211,7 @@
 
   /* Return the result.  If there is only one single Lisp object,
      return it as-it-is, otherwise return the reversed list.  */
-  if (XUINT (Flength (result)) == 1)
+  if (XFASTINT (Flength (result)) == 1)
     RETURN_UNGCPRO (CAR_SAFE (result));
   else
     RETURN_UNGCPRO (Fnreverse (result));
@@ -1251,6 +1276,7 @@
   DBusMessage *dmessage;
   DBusMessageIter iter;
   unsigned int dtype;
+  dbus_uint32_t serial;
   int timeout = -1;
   size_t i = 6;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
@@ -1292,7 +1318,7 @@
   if ((i+2 <= nargs) && (EQ ((args[i]), QCdbus_timeout)))
     {
       CHECK_NATNUM (args[i+1]);
-      timeout = XUINT (args[i+1]);
+      timeout = XFASTINT (args[i+1]);
       i = i+2;
     }
 
@@ -1335,7 +1361,8 @@
        XD_SIGNAL1 (build_string ("Cannot send message"));
 
       /* The result is the key in Vdbus_registered_objects_table.  */
-      result = (list2 (bus, make_number (dbus_message_get_serial (dmessage))));
+      serial = dbus_message_get_serial (dmessage);
+      result = list2 (bus, make_fixnum_or_float (serial));
 
       /* Create a hash table entry.  */
       Fputhash (result, handler, Vdbus_registered_objects_table);
@@ -1368,25 +1395,26 @@
 usage: (dbus-method-return-internal BUS SERIAL SERVICE &rest ARGS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  Lisp_Object bus, serial, service;
-  struct gcpro gcpro1, gcpro2, gcpro3;
+  Lisp_Object bus, service;
+  struct gcpro gcpro1, gcpro2;
   DBusConnection *connection;
   DBusMessage *dmessage;
   DBusMessageIter iter;
-  unsigned int dtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial, dtype;
   size_t i;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
 
   /* Check parameters.  */
   bus = args[0];
-  serial = args[1];
   service = args[2];
 
-  CHECK_NUMBER (serial);
+  CHECK_DBUS_SERIAL_GET_SERIAL (args[1], serial);
   CHECK_STRING (service);
-  GCPRO3 (bus, serial, service);
+  GCPRO2 (bus, service);
 
-  XD_DEBUG_MESSAGE ("%"pI"u %s ", XUINT (serial), SDATA (service));
+  ui_serial = serial;
+  XD_DEBUG_MESSAGE ("%u %s ", ui_serial, SSDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus, TRUE);
@@ -1394,7 +1422,7 @@
   /* Create the message.  */
   dmessage = dbus_message_new (DBUS_MESSAGE_TYPE_METHOD_RETURN);
   if ((dmessage == NULL)
-      || (!dbus_message_set_reply_serial (dmessage, XUINT (serial)))
+      || (!dbus_message_set_reply_serial (dmessage, serial))
       || (!dbus_message_set_destination (dmessage, SSDATA (service))))
     {
       UNGCPRO;
@@ -1456,25 +1484,26 @@
 usage: (dbus-method-error-internal BUS SERIAL SERVICE &rest ARGS)  */)
   (size_t nargs, register Lisp_Object *args)
 {
-  Lisp_Object bus, serial, service;
-  struct gcpro gcpro1, gcpro2, gcpro3;
+  Lisp_Object bus, service;
+  struct gcpro gcpro1, gcpro2;
   DBusConnection *connection;
   DBusMessage *dmessage;
   DBusMessageIter iter;
-  unsigned int dtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial, dtype;
   size_t i;
   char signature[DBUS_MAXIMUM_SIGNATURE_LENGTH];
 
   /* Check parameters.  */
   bus = args[0];
-  serial = args[1];
   service = args[2];
 
-  CHECK_NUMBER (serial);
+  CHECK_DBUS_SERIAL_GET_SERIAL (args[1], serial);
   CHECK_STRING (service);
-  GCPRO3 (bus, serial, service);
+  GCPRO2 (bus, service);
 
-  XD_DEBUG_MESSAGE ("%"pI"u %s ", XUINT (serial), SDATA (service));
+  ui_serial = serial;
+  XD_DEBUG_MESSAGE ("%u %s ", ui_serial, SSDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus, TRUE);
@@ -1483,7 +1512,7 @@
   dmessage = dbus_message_new (DBUS_MESSAGE_TYPE_ERROR);
   if ((dmessage == NULL)
       || (!dbus_message_set_error_name (dmessage, DBUS_ERROR_FAILED))
-      || (!dbus_message_set_reply_serial (dmessage, XUINT (serial)))
+      || (!dbus_message_set_reply_serial (dmessage, serial))
       || (!dbus_message_set_destination (dmessage, SSDATA (service))))
     {
       UNGCPRO;
@@ -1663,7 +1692,9 @@
   DBusMessage *dmessage;
   DBusMessageIter iter;
   unsigned int dtype;
-  int mtype, serial;
+  int mtype;
+  dbus_uint32_t serial;
+  unsigned int ui_serial;
   const char *uname, *path, *interface, *member;
 
   dmessage = dbus_connection_pop_message (connection);
@@ -1692,7 +1723,7 @@
   /* Read message type, message serial, unique name, object path,
      interface and member from the message.  */
   mtype = dbus_message_get_type (dmessage);
-  serial =
+  ui_serial = serial =
     ((mtype == DBUS_MESSAGE_TYPE_METHOD_RETURN)
      || (mtype == DBUS_MESSAGE_TYPE_ERROR))
     ? dbus_message_get_reply_serial (dmessage)
@@ -1702,7 +1733,7 @@
   interface = dbus_message_get_interface (dmessage);
   member = dbus_message_get_member (dmessage);
 
-  XD_DEBUG_MESSAGE ("Event received: %s %d %s %s %s %s %s",
+  XD_DEBUG_MESSAGE ("Event received: %s %u %s %s %s %s %s",
                    (mtype == DBUS_MESSAGE_TYPE_INVALID)
                    ? "DBUS_MESSAGE_TYPE_INVALID"
                    : (mtype == DBUS_MESSAGE_TYPE_METHOD_CALL)
@@ -1712,14 +1743,14 @@
                    : (mtype == DBUS_MESSAGE_TYPE_ERROR)
                    ? "DBUS_MESSAGE_TYPE_ERROR"
                    : "DBUS_MESSAGE_TYPE_SIGNAL",
-                   serial, uname, path, interface, member,
+                   ui_serial, uname, path, interface, member,
                    SDATA (format2 ("%s", args, Qnil)));
 
   if ((mtype == DBUS_MESSAGE_TYPE_METHOD_RETURN)
       || (mtype == DBUS_MESSAGE_TYPE_ERROR))
     {
       /* Search for a registered function of the message.  */
-      key = list2 (bus, make_number (serial));
+      key = list2 (bus, make_fixnum_or_float (serial));
       value = Fgethash (key, Vdbus_registered_objects_table, Qnil);
 
       /* There shall be exactly one entry.  Construct an event.  */
@@ -1785,7 +1816,7 @@
                     event.arg);
   event.arg = Fcons ((uname == NULL ? Qnil : build_string (uname)),
                     event.arg);
-  event.arg = Fcons (make_number (serial), event.arg);
+  event.arg = Fcons (make_fixnum_or_float (serial), event.arg);
   event.arg = Fcons (make_number (mtype), event.arg);
 
   /* Add the bus symbol to the event.  */




--- End Message ---
--- Begin Message --- Subject: fixes committed to trunk Date: Fri, 27 May 2011 13:40:10 -0700 User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc14 Thunderbird/3.1.10
I just committed bzr 104390 to the Emacs trunk.
It merges fixes for bugs 8668, 8719, and 8722 as
previously discussed.

For Bug#8719, although the patch should fix the problems
it may not be the best patch.  Kenichi Handa wrote that
he'll check it.  Kenichi, if there are problems with it,
please feel free to replace it with something better
or to send me email with suggestions and I'll work on
making it better.  Thanks.


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]