[PATCH] Trivial segfaults in Python bindings
Andrew Bennetts
andrew-dbus@puzzling.org
Sat, 31 Jan 2004 00:09:27 +1100
--envbJBWh7q8WU6mo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Currently, the Python bindings assume that it's ok to cast random structs
like DBusMessage * to PyObject *, even though Pyrex will likely attempt to
do all sorts of nasty refcounting calls on the PyObject * structure, which
is likely to cause all sorts of nasty problems. The _get_* methods are
fundamentally broken because of this.
Also, the Python bindings don't do enough sanity-checking of arguments to
ensure that NULL pointers aren't exposed to Python.
Here are some examples of ways to segfault the current Python bindings:
import dbus_bindings
print dbus_bindings.Message()
print dbus_bindings.Message(message_type=1, path='x', interface='y',
method='z')._get_msg()
print dbus_bindings.Message(message_type=2)
(I don't care if these calls are nonsensical; they shouldn't crash Python)
The attached patch fixes these problems on the Message class. Please apply
it. I'll send patches for the rest when I've finished looking at them.
Regards,
-Andrew.
--envbJBWh7q8WU6mo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="dbus_python_message.patch"
Index: python/dbus_bindings.pyx.in
===================================================================
RCS file: /cvs/dbus/dbus/python/dbus_bindings.pyx.in,v
retrieving revision 1.4
diff -u -r1.4 dbus_bindings.pyx.in
--- python/dbus_bindings.pyx.in 29 Oct 2003 00:06:07 -0000 1.4
+++ python/dbus_bindings.pyx.in 30 Jan 2004 13:10:36 -0000
@@ -17,6 +17,12 @@
void Py_XINCREF (object)
void Py_XDECREF (object)
+ int PyCObject_Check(object p)
+ object PyCObject_FromVoidPtr(void* cobj, void (*destr)(void *))
+ object PyCObject_FromVoidPtrAndDesc(void* cobj, void* desc,
+ void (*destr)(void *, void *))
+ void* PyCObject_AsVoidPtr(object self)
+ void* PyCObject_GetDesc(object self)
ctypedef struct DBusError:
char *name
@@ -75,8 +81,9 @@
tup = <object>user_data
assert (type(tup) == list)
function = tup[0]
- message = Message(_create=0)
- message._set_msg(<object>msg)
+ #message = Message(_create=0)
+ #message._set_msg(<object>msg)
+ message = Message(msg=PyCObject_FromVoidPtr(msg, NULL))
args = [Connection(_conn=<object>connection),
message]
retval = function(*args)
@@ -84,6 +91,251 @@
retval = DBUS_HANDLER_RESULT_HANDLED
return retval
+(MESSAGE_TYPE_INVALID, MESSAGE_TYPE_METHOD_CALL, MESSAGE_TYPE_METHOD_RETURN, MESSAGE_TYPE_ERROR, MESSAGE_TYPE_SIGNAL) = range(5)
+(TYPE_INVALID, TYPE_NIL, TYPE_BYTE, TYPE_BOOLEAN, TYPE_INT32, TYPE_UINT32, TYPE_INT64, TYPE_UINT64, TYPE_DOUBLE, TYPE_STRING, TYPE_CUSTOM, TYPE_ARRAY, TYPE_DICT, TYPE_OBJECT_PATH) = (0, ord('v'), ord('y'), ord('b'), ord('i'), ord('u'), ord('x'), ord('t'), ord('d'), ord('s'), ord('c'), ord('a'), ord('m'), ord('o'))
+
+cdef class Message:
+ cdef DBusMessage *msg
+
+ def __init__(self, message_type=MESSAGE_TYPE_INVALID,
+ service=None, path=None, interface=None, method=None,
+ method_call=None,
+ name=None,
+ reply_to=None, error_name=None, error_message=None,
+ msg=None):
+ cdef char *cservice
+ cdef Message creply_to
+ cdef Message cmethod_call
+ if (service == None):
+ cservice = NULL
+ else:
+ cservice = service
+
+ if reply_to is not None:
+ creply_to = reply_to
+ if method_call is not None:
+ cmethod_call = method_call
+
+ if msg is not None:
+ if not PyCObject_Check(msg):
+ raise TypeError, "msg must be a CObject or None, not %r" \
+ % (msg,)
+ self.msg = <DBusMessage *> PyCObject_AsVoidPtr(msg)
+ dbus_message_ref(self.msg)
+ return
+
+ if message_type == MESSAGE_TYPE_METHOD_CALL:
+ self.msg = dbus_message_new_method_call(cservice, path, interface, method)
+ elif message_type == MESSAGE_TYPE_METHOD_RETURN:
+ if method_call is None:
+ raise TypeError, "method_call must be an instance of Message " \
+ "when message_type is " \
+ "MESSAGE_TYPE_METHOD_RETURN"
+ self.msg = dbus_message_new_method_return(cmethod_call.msg)
+ elif message_type == MESSAGE_TYPE_SIGNAL:
+ self.msg = dbus_message_new_signal(path, interface, name)
+ elif message_type == MESSAGE_TYPE_ERROR:
+ if reply_to is None:
+ raise TypeError, "reply_to must be an instance of Message " \
+ "when message_type is MESSAGE_TYPE_ERROR"
+ self.msg = dbus_message_new_error(creply_to.msg, error_name, error_message)
+ else:
+ raise DBusException, "Unknown message type: %r" % (message_type,)
+
+ def __dealloc__(self):
+ if self.msg != NULL:
+ dbus_message_unref(self.msg)
+
+ def type_to_name(self, type):
+ if type == MESSAGE_TYPE_SIGNAL:
+ return "signal"
+ elif type == MESSAGE_TYPE_METHOD_CALL:
+ return "method call"
+ elif type == MESSAGE_TYPE_METHOD_RETURN:
+ return "method return"
+ elif type == MESSAGE_TYPE_ERROR:
+ return "error"
+ else:
+ return "(unknown message type)"
+
+ def __str__(self):
+ message_type = self.get_type()
+ sender = self.get_sender()
+
+ if sender == None:
+ sender = "(no sender)"
+
+ if (message_type == MESSAGE_TYPE_METHOD_CALL) or (message_type == MESSAGE_TYPE_SIGNAL):
+ retval = '%s interface=%s; member=%s; sender=%s' % (self.type_to_name(message_type),
+ self.get_interface(),
+ self.get_member(),
+ sender)
+ elif message_type == MESSAGE_TYPE_METHOD_RETURN:
+ retval = '%s sender=%s' % (self.type_to_name(message_type),
+ sender)
+ elif message_type == MESSAGE_TYPE_ERROR:
+ retval = '%s name=%s; sender=%s' % (self.type_to_name(message_type),
+ self.get_error_name(),
+ sender)
+ else:
+ retval = "Message of unknown type %d" % (message_type)
+
+
+ # FIXME: should really use self.convert_to_tuple() here
+
+ iter = self.get_iter()
+ value_at_iter = True
+
+ while (value_at_iter):
+ type = iter.get_arg_type()
+
+ if type == TYPE_INVALID:
+ break
+ elif type == TYPE_STRING:
+ str = iter.get_string()
+ arg = 'string:%s\n' % (str)
+ elif type == TYPE_INT32:
+ num = iter.get_int32()
+ arg = 'int32:%d\n' % (num)
+ elif type == TYPE_UINT32:
+ num = iter.get_uint32()
+ arg = 'uint32:%u\n' % (num)
+ elif type == TYPE_DOUBLE:
+ num = iter.get_double()
+ arg = 'double:%f\n' % (num)
+ elif type == TYPE_BYTE:
+ num = iter.get_byte()
+ arg = 'byte:%d\n' % (num)
+ elif type == TYPE_BOOLEAN:
+ bool = iter.get_boolean()
+ if (bool):
+ str = "true"
+ else:
+ str = "false"
+ arg = 'boolean:%s\n' % (str)
+ else:
+ arg = '(unknown arg type %d)\n' % type
+
+ retval = retval + arg
+ value_at_iter = iter.next()
+
+ return retval
+
+ def get_iter(self):
+ return MessageIter(self)
+
+ def get_args_list(self):
+ retval = [ ]
+
+ iter = self.get_iter()
+ try:
+ retval.append(iter.get())
+ except TypeError, e:
+ return [ ]
+
+ value_at_iter = iter.next()
+ while (value_at_iter):
+ retval.append(iter.get())
+ value_at_iter = iter.next()
+
+ return retval
+
+ # FIXME: implement dbus_message_copy?
+
+ def get_type(self):
+ return dbus_message_get_type(self.msg)
+
+ def set_path(self, object_path):
+ return dbus_message_set_path(self.msg, object_path)
+
+ def get_path(self):
+ return dbus_message_get_path(self.msg)
+
+ def set_interface(self, interface):
+ return dbus_message_set_interface(self.msg, interface)
+
+ def get_interface(self):
+ return dbus_message_get_interface(self.msg)
+
+ def set_member(self, member):
+ return dbus_message_set_member(self.msg, member)
+
+ def get_member(self):
+ return dbus_message_get_member(self.msg)
+
+ def set_error_name(self, name):
+ return dbus_message_set_error_name(self.msg, name)
+
+ def get_error_name(self):
+ return dbus_message_get_error_name(self.msg)
+
+ def set_destination(self, destination):
+ return dbus_message_set_destination(self.msg, destination)
+
+ def get_destination(self):
+ return dbus_message_get_destination(self.msg)
+
+ def set_sender(self, sender):
+ return dbus_message_set_sender(self.msg, sender)
+
+ def get_sender(self):
+ cdef char *sender
+ sender = dbus_message_get_sender(self.msg)
+ if (sender == NULL):
+ return None
+ else:
+ return sender
+
+ def set_no_reply(self, no_reply):
+ dbus_message_set_no_reply(self.msg, no_reply)
+
+ def get_no_reply(self):
+ return dbus_message_get_no_reply(self.msg)
+
+ def is_method_call(self, interface, method):
+ return dbus_message_is_method_call(self.msg, interface, method)
+
+ def is_signal(self, interface, signal_name):
+ return dbus_message_is_signal(self.msg, interface, signal_name)
+
+ def is_error(self, error_name):
+ return dbus_message_is_error(self.msg, error_name)
+
+ def has_destination(self, service):
+ return dbus_message_has_destination(self.msg, service)
+
+ def has_sender(self, service):
+ return dbus_message_has_sender(self.msg, service)
+
+ def get_serial(self):
+ return dbus_message_get_serial(self.msg)
+
+ def set_reply_serial(self, reply_serial):
+ return dbus_message_set_reply_serial(self.msg, reply_serial)
+
+ def get_reply_serial(self):
+ return dbus_message_get_reply_serial(self.msg)
+
+ #FIXME: dbus_message_get_path_decomposed
+
+ # FIXME: all the different dbus_message_*args* methods
+
+class Signal(Message):
+ def __init__(self, spath, sinterface, sname):
+ Message.__init__(self, MESSAGE_TYPE_SIGNAL, path=spath, interface=sinterface, name=sname)
+
+class MethodCall(Message):
+ def __init__(self, mpath, minterface, mmethod):
+ Message.__init__(self, MESSAGE_TYPE_METHOD_CALL, path=mpath, interface=minterface, method=mmethod)
+
+class MethodReturn(Message):
+ def __init__(self, method_call):
+ Message.__init__(self, MESSAGE_TYPE_METHOD_RETURN, method_call=method_call)
+
+class Error(Message):
+ def __init__(self, reply_to, error_name, error_message):
+ Message.__init__(self, MESSAGE_TYPE_ERROR, reply_to=reply_to, error_name=error_name, error_message=error_message)
+
cdef class Connection:
cdef DBusConnection *conn
@@ -144,25 +396,29 @@
dbus_connection_flush(self.conn)
def borrow_message(self):
- m = Message(_create=0)
- m._set_msg(<object>dbus_connection_borrow_message(self.conn))
+ cdef Message m
+ cdef DBusMessage * msg
+ msg = dbus_connection_borrow_message(self.conn)
+ m = Message(msg=PyCObject_FromVoidPtr(msg, NULL))
return m
def return_message(self, message):
- msg = message._get_msg()
- dbus_connection_return_message(self.conn, <DBusMessage*>msg)
+ cdef Message cmessage
+ cmessage = message
+ dbus_connection_return_message(self.conn, cmessage.msg)
def steal_borrowed_message(self, message):
- msg = message._get_msg()
- dbus_connection_steal_borrowed_message(self.conn,
- <DBusMessage*>msg)
+ cdef Message cmessage
+ cdef DBusMessage * msg
+ cmessage = message
+ msg = cmessage.msg
+ dbus_connection_steal_borrowed_message(self.conn, msg)
def pop_message(self):
cdef DBusMessage *msg
msg = dbus_connection_pop_message(self.conn)
if msg != NULL:
- m = Message(_create=0)
- m._set_msg(<object>msg)
+ m = Message(msg=PyCObject_FromVoidPtr(msg, NULL))
else:
m = None
return m
@@ -178,24 +434,28 @@
#if type(message) != Message:
# raise TypeError
- msg = message._get_msg()
- retval = dbus_connection_send(self.conn,
- <DBusMessage*>msg,
- NULL)
+ cdef Message cmessage
+ cdef DBusMessage * msg
+ cmessage = message
+ msg = cmessage.msg
+ retval = dbus_connection_send(self.conn, msg, NULL)
return retval
def send_with_reply(self, message, timeout_milliseconds):
cdef dbus_bool_t retval
cdef DBusPendingCall *cpending_call
cdef DBusError error
+ cdef Message cmessage
+ cdef DBusMessage * msg
dbus_error_init(&error)
cpending_call = NULL
- msg = message._get_msg()
+ cmessage = message
+ msg = cmessage.msg
retval = dbus_connection_send_with_reply(self.conn,
- <DBusMessage*>msg,
+ msg,
&cpending_call,
timeout_milliseconds)
@@ -213,13 +473,16 @@
timeout_milliseconds=0):
cdef DBusMessage * retval
cdef DBusError error
+ cdef Message cmessage, m
+ cdef DBusMessage * msg
dbus_error_init(&error)
- msg = message._get_msg()
+ cmessage = message
+ msg = cmessage.msg
retval = dbus_connection_send_with_reply_and_block(
<DBusConnection*>self.conn,
- <DBusMessage*>msg,
+ msg,
<int>timeout_milliseconds,
&error)
@@ -229,8 +492,7 @@
if retval == NULL:
raise AssertionError
- m = Message(_create=0)
- m._set_msg(<object>retval)
+ m = Message(msg=PyCObject_FromVoidPtr(msg, NULL))
return m
def set_watch_functions(self, add_function, remove_function, data):
@@ -358,8 +620,10 @@
return dbus_pending_call_get_completed(self.pending_call)
def get_reply(self):
- message = Message(_create=0)
- message._set_msg(<object>dbus_pending_call_get_reply(self.pending_call))
+ cdef DBusMessage * msg
+ cdef Message message
+ msg = dbus_pending_call_get_reply(self.pending_call)
+ message = Message(msg=PyCObject_FromVoidPtr(msg, NULL))
return message
def block(self):
@@ -389,9 +653,10 @@
def __init__(self, message):
+ cdef Message cmessage
+ cmessage = message
self.iter = &self.real_iter
- msg = message._get_msg()
- dbus_message_iter_init(<DBusMessage*>msg, self.iter)
+ dbus_message_iter_init(cmessage.msg, self.iter)
def get_iter(self):
return <object>self.iter
@@ -591,234 +856,6 @@
return dbus_message_iter_append_string_array(self.iter, value, length)
-(MESSAGE_TYPE_INVALID, MESSAGE_TYPE_METHOD_CALL, MESSAGE_TYPE_METHOD_RETURN, MESSAGE_TYPE_ERROR, MESSAGE_TYPE_SIGNAL) = range(5)
-(TYPE_INVALID, TYPE_NIL, TYPE_BYTE, TYPE_BOOLEAN, TYPE_INT32, TYPE_UINT32, TYPE_INT64, TYPE_UINT64, TYPE_DOUBLE, TYPE_STRING, TYPE_CUSTOM, TYPE_ARRAY, TYPE_DICT, TYPE_OBJECT_PATH) = (0, ord('v'), ord('y'), ord('b'), ord('i'), ord('u'), ord('x'), ord('t'), ord('d'), ord('s'), ord('c'), ord('a'), ord('m'), ord('o'))
-
-cdef class Message:
- cdef DBusMessage *msg
-
- def __init__(self, message_type=MESSAGE_TYPE_INVALID,
- service=None, path=None, interface=None, method=None,
- method_call=None,
- name=None,
- reply_to=None, error_name=None, error_message=None,
- _create=1):
- cdef char *cservice
- if (service == None):
- cservice = NULL
- else:
- cservice = service
-
- if not _create:
- return
-
- if message_type == MESSAGE_TYPE_METHOD_CALL:
- self.msg = dbus_message_new_method_call(cservice, path, interface, method)
- elif message_type == MESSAGE_TYPE_METHOD_RETURN:
- cmsg = method_call._get_msg()
- self.msg = dbus_message_new_method_return(<DBusMessage*>cmsg)
- elif message_type == MESSAGE_TYPE_SIGNAL:
- self.msg = dbus_message_new_signal(path, interface, name)
- elif message_type == MESSAGE_TYPE_ERROR:
- cmsg = reply_to._get_msg()
- self.msg = dbus_message_new_error(<DBusMessage*>cmsg, error_name, error_message)
-
- def type_to_name(self, type):
- if type == MESSAGE_TYPE_SIGNAL:
- return "signal"
- elif type == MESSAGE_TYPE_METHOD_CALL:
- return "method call"
- elif type == MESSAGE_TYPE_METHOD_RETURN:
- return "method return"
- elif type == MESSAGE_TYPE_ERROR:
- return "error"
- else:
- return "(unknown message type)"
-
- def __str__(self):
- message_type = self.get_type()
- sender = self.get_sender()
-
- if sender == None:
- sender = "(no sender)"
-
- if (message_type == MESSAGE_TYPE_METHOD_CALL) or (message_type == MESSAGE_TYPE_SIGNAL):
- retval = '%s interface=%s; member=%s; sender=%s' % (self.type_to_name(message_type),
- self.get_interface(),
- self.get_member(),
- sender)
- elif message_type == MESSAGE_TYPE_METHOD_RETURN:
- retval = '%s sender=%s' % (self.type_to_name(message_type),
- sender)
- elif message_type == MESSAGE_TYPE_ERROR:
- retval = '%s name=%s; sender=%s' % (self.type_to_name(message_type),
- self.get_error_name(),
- sender)
- else:
- retval = "Message of unknown type %d" % (message_type)
-
-
- # FIXME: should really use self.convert_to_tuple() here
-
- iter = self.get_iter()
- value_at_iter = True
-
- while (value_at_iter):
- type = iter.get_arg_type()
-
- if type == TYPE_INVALID:
- break
- elif type == TYPE_STRING:
- str = iter.get_string()
- arg = 'string:%s\n' % (str)
- elif type == TYPE_INT32:
- num = iter.get_int32()
- arg = 'int32:%d\n' % (num)
- elif type == TYPE_UINT32:
- num = iter.get_uint32()
- arg = 'uint32:%u\n' % (num)
- elif type == TYPE_DOUBLE:
- num = iter.get_double()
- arg = 'double:%f\n' % (num)
- elif type == TYPE_BYTE:
- num = iter.get_byte()
- arg = 'byte:%d\n' % (num)
- elif type == TYPE_BOOLEAN:
- bool = iter.get_boolean()
- if (bool):
- str = "true"
- else:
- str = "false"
- arg = 'boolean:%s\n' % (str)
- else:
- arg = '(unknown arg type %d)\n' % type
-
- retval = retval + arg
- value_at_iter = iter.next()
-
- return retval
-
- def _set_msg(self, msg):
- self.msg = <DBusMessage*>msg
-
- def _get_msg(self):
- return <object>self.msg
-
- def get_iter(self):
- return MessageIter(self)
-
- def get_args_list(self):
- retval = [ ]
-
- iter = self.get_iter()
- try:
- retval.append(iter.get())
- except TypeError, e:
- return [ ]
-
- value_at_iter = iter.next()
- while (value_at_iter):
- retval.append(iter.get())
- value_at_iter = iter.next()
-
- return retval
-
- # FIXME: implement dbus_message_copy?
-
- def get_type(self):
- return dbus_message_get_type(self.msg)
-
- def set_path(self, object_path):
- return dbus_message_set_path(self.msg, object_path)
-
- def get_path(self):
- return dbus_message_get_path(self.msg)
-
- def set_interface(self, interface):
- return dbus_message_set_interface(self.msg, interface)
-
- def get_interface(self):
- return dbus_message_get_interface(self.msg)
-
- def set_member(self, member):
- return dbus_message_set_member(self.msg, member)
-
- def get_member(self):
- return dbus_message_get_member(self.msg)
-
- def set_error_name(self, name):
- return dbus_message_set_error_name(self.msg, name)
-
- def get_error_name(self):
- return dbus_message_get_error_name(self.msg)
-
- def set_destination(self, destination):
- return dbus_message_set_destination(self.msg, destination)
-
- def get_destination(self):
- return dbus_message_get_destination(self.msg)
-
- def set_sender(self, sender):
- return dbus_message_set_sender(self.msg, sender)
-
- def get_sender(self):
- cdef char *sender
- sender = dbus_message_get_sender(self.msg)
- if (sender == NULL):
- return None
- else:
- return sender
-
- def set_no_reply(self, no_reply):
- dbus_message_set_no_reply(self.msg, no_reply)
-
- def get_no_reply(self):
- return dbus_message_get_no_reply(self.msg)
-
- def is_method_call(self, interface, method):
- return dbus_message_is_method_call(self.msg, interface, method)
-
- def is_signal(self, interface, signal_name):
- return dbus_message_is_signal(self.msg, interface, signal_name)
-
- def is_error(self, error_name):
- return dbus_message_is_error(self.msg, error_name)
-
- def has_destination(self, service):
- return dbus_message_has_destination(self.msg, service)
-
- def has_sender(self, service):
- return dbus_message_has_sender(self.msg, service)
-
- def get_serial(self):
- return dbus_message_get_serial(self.msg)
-
- def set_reply_serial(self, reply_serial):
- return dbus_message_set_reply_serial(self.msg, reply_serial)
-
- def get_reply_serial(self):
- return dbus_message_get_reply_serial(self.msg)
-
- #FIXME: dbus_message_get_path_decomposed
-
- # FIXME: all the different dbus_message_*args* methods
-
-class Signal(Message):
- def __init__(self, spath, sinterface, sname):
- Message.__init__(self, MESSAGE_TYPE_SIGNAL, path=spath, interface=sinterface, name=sname)
-
-class MethodCall(Message):
- def __init__(self, mpath, minterface, mmethod):
- Message.__init__(self, MESSAGE_TYPE_METHOD_CALL, path=mpath, interface=minterface, method=mmethod)
-
-class MethodReturn(Message):
- def __init__(self, method_call):
- Message.__init__(self, MESSAGE_TYPE_METHOD_RETURN, method_call=method_call)
-
-class Error(Message):
- def __init__(self, reply_to, error_name, error_message):
- Message.__init__(self, MESSAGE_TYPE_ERROR, reply_to=reply_to, error_name=error_name, error_message=error_message)
-
cdef class Server:
cdef DBusServer *server
def __init__(self, address):
--envbJBWh7q8WU6mo--