* bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow
@ 2011-05-24 5:32 Paul Eggert
2011-05-24 7:04 ` Michael Albinus
0 siblings, 1 reply; 3+ messages in thread
From: Paul Eggert @ 2011-05-24 5:32 UTC (permalink / raw)
To: 8722
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 <eggert@cs.ucla.edu>
+ * 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 <eggert@cs.ucla.edu>
=== 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. */
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow
2011-05-24 5:32 bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow Paul Eggert
@ 2011-05-24 7:04 ` Michael Albinus
2011-05-24 7:42 ` Paul Eggert
0 siblings, 1 reply; 3+ messages in thread
From: Michael Albinus @ 2011-05-24 7:04 UTC (permalink / raw)
To: Paul Eggert; +Cc: 8722
Paul Eggert <eggert@cs.ucla.edu> writes:
> 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.
On a raw look, it seems to be OK. Minor remark: Why do you need
xd_invalid_serial? You could call instead
XD_SIGNAL2 (build_string ("Invalid dbus serial"), x);
which returns a `dbus-error', and which is more sensible when reading
DBus events.
Best regards, Michael.
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow
2011-05-24 7:04 ` Michael Albinus
@ 2011-05-24 7:42 ` Paul Eggert
0 siblings, 0 replies; 3+ messages in thread
From: Paul Eggert @ 2011-05-24 7:42 UTC (permalink / raw)
To: Michael Albinus; +Cc: 8722
On 05/24/11 00:04, Michael Albinus wrote:
> Why do you need xd_invalid_serial? You could call instead
>
> XD_SIGNAL2 (build_string ("Invalid dbus serial"), x);
Thanks, I didn't think of that. I'll make that change.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-05-24 7:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-24 5:32 bug#8722: dbusbind.c fixes for integer issues, e.g., integer overflow Paul Eggert
2011-05-24 7:04 ` Michael Albinus
2011-05-24 7:42 ` Paul Eggert
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).