D-Bus object path and interface must be validate otherwise it can cause
errors as follow:
0 0xb7f67ab1 in __kernel_vsyscall ()
1 0xb7ca1cc1 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
2 0xb7ca50ee in abort () at abort.c:92
3 0xb7e30ba5 in _dbus_abort () at dbus-sysdeps.c:94
4 0xb7e267a6 in _dbus_warn_check_failed (
format=0xb7e36cd4 "arguments to %s() were incorrect, assertion \"%s\" failed in file %s line %d.\nThis is normally a bug in some application using the D-Bus library.\n") at dbus-internals.c:290
5 0xb7e16d9f in dbus_message_iter_append_basic (iter=0xbf864400, type=111,
value=0xd70940) at dbus-message.c:2586
6 0x004fcdec in emit_interfaces_added (user_data=0xd70938)
at gdbus/object.c:574
This should make it safe to call g_dbus_send_message with a NULL
message which is more likely to happen now that g_dbus_create_reply
returns NULL when the message is marked with NO_REPLY flag.
When a sender flags a D-Bus message as not expecting a reply, it is
against D-Bus policy to send a reply — sending one can result in an
error as reported in:
https://bugzilla.kernel.org/show_bug.cgi?id=198453
g_dbus_proxy_path_lookup() returns the path of the proxy that matches the
passed path. It also returns the index of the proxy coming next to the
matched proxy in the passed list.
Also adds the following feature to g_dbus_proxy_lookup().
- It is more robust even if a proxy is NULL.
- It checks if the passed interface is NULL.
- It looks up from the position of the list specified by the index.
If proxy was freed due to interface being removed remaining references
are left with NULL client pointer. We need to cancel pending calls that
require client when getting reply.
This fix following crash:
bluetoothd[2773]: src/gatt-database.c:proxy_removed_cb() Proxy removed - removing service: /test/app/hci0/service2
bluetoothd[2773]: src/gatt-database.c:gatt_db_service_removed() Local GATT service removed
bluetoothd[2773]: src/adapter.c:adapter_service_remove() /org/bluez/hci0
bluetoothd[2773]: src/adapter.c:remove_uuid() sending remove uuid command for index 0
bluetoothd[2773]: src/sdpd-service.c:remove_record_from_server() Removing record with handle 0x10008
bluetoothd[2773]: src/gatt-database.c:client_disconnect_cb() Client disconnected
==2773== Invalid read of size 8
==2773== at 0x485220: proxy_added (client.c:288)
==2773== by 0x485220: get_all_properties_reply (client.c:316)
==2773== by 0x515A041: ??? (in /lib/x86_64-linux-gnu/libdbus-1.so.3.14.6)
==2773== by 0x515DA60: dbus_connection_dispatch (in /lib/x86_64-linux-gnu/libdbus-1.so.3.14.6)
==2773== by 0x47F2BF: message_dispatch (mainloop.c:72)
==2773== by 0x4E84049: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==2773== by 0x4E843EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==2773== by 0x4E84711: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4800.2)
==2773== by 0x40B51F: main (main.c:770)
==2773== Address 0x88 is not stack'd, malloc'd or (recently) free'd
If and interface is removed while properties are pending it would cause
process_properties_from_interface to clear data->pending_prop when it
should only clear the iface->pending_prop.
Changes introduced by ef024c2c44 have a
regression were a proxy may not be appended to the proxy_list causing
tests to fail as the proxies maybe be added multiple time since
proxy_look would not be able to find existing instances.
Consider the following example:
/foo
properties: "A", "B"
/bar
properties: "C", "D"
If during a given mainloop iteration, property "A" of object '/foo' is
changed, then properties "C" and "D" of '/bar', lastly "B" of '/foo',
the current code will emit the PropertiesChanged signals in following
order: "A", "B", "C", "D".
This may confuse applications that have a dependency on the order of
those signals.
This fixes the ordering, so in the example, the order becomes:
"C", "D", "A", B". This is considered not to be a problem, as
applications may use the flag G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH, so
property changed signals are emitted as soon as possible.
The solution is for each object, to reschedule the signals every time a
signal is emitted.
Call ready callback regardless of the reply to GetManagedObjects
since otherwise the user code will be left waiting forever when in fact
no proxy will be created.
Move enumeration type defintions AFTER the enumerations themselves are
declared and defined such that the header works with strict compilers.
This occurs when building a plugin, compiled with C++, and occurs on
all of:
arm-none-linux-gnueabi-g++ (Sourcery G++ Lite 2010q1-202) 4.4.1
arm-poky-linux-gnueabi-g++ (GCC) 4.8.2
g++ (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4
If the sender flags a D-Bus message as not expecting a reply, it is
against system bus policy to send a reply — sending one will result in
errors being sent to us by dbus-daemon.
Magically drop all replies to messages which request no reply.
ofonod is one process which sends BlueZ messages which request no
reply,
when it is setting up a hands-free agent.
This is not a complete fix. In an ideal world, the existing check for
G_DBUS_METHOD_FLAG_NOREPLY would be dropped, as the server should be
prepared to return a reply to every method, if the client requests and
expects one — otherwise the client will time out. However, that’s a
much
bigger change with a much bigger risk of breaking things, so I’ll stick
with this for now.
g_dbus_emit_property_changed doesn't send dbus signal immediately. Instead
it stores changed properties, and schedule signal to be send at
g_iddle_add. Additionally, if this method is called few times for some
property, only last value will be sent in property changed signal.
If remote device sends lots of notifications, they're all scheduled to be
notified using this method. This might result in some notifications being
lost.
This patch adds new method, that can immediately send property changed
signal, instead of sheduling it for nearest iddle moment.
Private connection should be properly closed with dbus_connection_close
otherwise libdbus exits with the following error:
'The last reference on a connection was dropped without closing the
connection. This is a bug in an application. See
dbus_connection_unref() documentation for details. Most likely, the
application was supposed to call dbus_connection_close(), since this
is a private connection.'
Passing NULL format parameter to vsnprintf results in invalid argument
error on glibc. But with some other libc libraries (musl and uClibc)
this results in dereferencing NULL pointer and crash due to
segmentation fault.
GDBusClient currently hard-codes "/" as the remote ObjectManager path.
This is generally incorrect, as an application can choose to expose an
ObjectManager at any well-known path. This patch fixes this by allowing
the user to pass in the ObjectManager path by introducing a new
conctructor "g_dbus_client_new_full".
The client code currently issues GetManagedObjects if new handlers are
set via g_dbus_client_set_proxy_handlers. An application may set these
to NULL before unref'ing a client or to simply prevent further events.
Hence, there is no need to refresh objects or properties if all handlers
are NULL.
If daemon gets disconnected from D-Bus sender is NULL. Watches that
was explicitly added with NULL sender (ie disconnected_signal in
g_dbus_set_disconnect_function) should be called anyway.
If g_dbus_client_set_proxy_handlers gets called from within a
proxy_removed callback, the code may end up refreshing the proxy's
properties and incorrectly access the client's proxy_list as it gets
freed. This patch fixes this, so that get_managed_objects does nothing
if it gets called during a service disconnect.
When subscribing to the D-Bus signal NameOwnerChanged from the bus driver,
specify the object path and the sender in the match rule. Otherwise, random
connections on the bus could impersonate the bus driver.
This partially reverts 510b32b715 since it
still necessary to take a reference before calling dbus_watch_handle
since internally it can call watch_info_free as in the following trace:
Invalid read of size 8
at 0x121085: watch_func (mainloop.c:105)
by 0x4C72694: g_main_context_dispatch (gmain.c:2539)
by 0x4C729C7: g_main_context_iterate.isra.23 (gmain.c:3146)
by 0x4C72DC1: g_main_loop_run (gmain.c:3340)
by 0x120541: main (main.c:551)
Address 0x5bbcd90 is 16 bytes inside a block of size 24 free'd
at 0x4A079AE: free (vg_replace_malloc.c:427)
by 0x4C7837E: g_free (gmem.c:252)
by 0x4F708BF: dbus_watch_set_data (dbus-watch.c:614)
by 0x4F70938: _dbus_watch_unref (dbus-watch.c:132)
by 0x4F6E9A7: _dbus_transport_handle_watch (dbus-transport.c:884)
by 0x4F59AFB: _dbus_connection_handle_watch (dbus-connection.c:1497)
by 0x4F70AF9: dbus_watch_handle (dbus-watch.c:683)
by 0x121084: watch_func (mainloop.c:103)
by 0x4C72694: g_main_context_dispatch (gmain.c:2539)
by 0x4C729C7: g_main_context_iterate.isra.23 (gmain.c:3146)
by 0x4C72DC1: g_main_loop_run (gmain.c:3340)
by 0x120541: main (main.c:551)
If g_dbus_add_service_watch is called for service which bus name is
already known the following crash can happen:
invalid read of size 1
at 0x4C2A2F2: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4E97722: g_strdup (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x405B0C: update_name_cache (watch.c:435)
by 0x405C37: update_service (watch.c:593)
by 0x4E7A2A5: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E7A627: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E7AA39: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4038EA: client_ready (test-gdbus-client.c:1014)
by 0x4E9E5E0: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E9E7A5: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E9EB1A: g_test_run_suite (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x403614: main (test-gdbus-client.c:1058)
Address 0x5dbe5d0 is 0 bytes inside a block of size 7 free'd
at 0x4C28577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4E7FF7E: g_free (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x405B04: update_name_cache (watch.c:434)
by 0x405C37: update_service (watch.c:593)
by 0x4E7A2A5: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E7A627: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E7AA39: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4038EA: client_ready (test-gdbus-client.c:1014)
by 0x4E9E5E0: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E9E7A5: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x4E9EB1A: g_test_run_suite (in /usr/lib64/libglib-2.0.so.0.3800.2)
by 0x403614: main (test-gdbus-client.c:1058)
This actually creates a problem since the code now does flush any pending
message on the queue any signal will be send right away before the
actual reply to GetManagedObjects which will not contain those interfaces.
No matter if disconnection was reported previously, g_dbus_client_unref()
was always calling service disconnect callback. This patch fix the
following scenario:
1) service disconnects from the bus
2) disconnect callback gets called
3) client calls g_dbus_client_unref(), disconnect callback is called
again.
This patch adds a new gdbus helper to notify the clients that
GetManagedObjects reply was received and the last proxy has been
informed previously by the proxy_added callback.
Commit abfc2b0dd5 attempted to fix a crash
related to improper reference counting, but the main issue was that the
reference was taken only during the function call (which is usually
unnecessary for single thread), but still passed a pointer to
DBusConnection to a function that is called by the mainloop. This left a
window where the DBusConnection can be destroyed.
Fixes this crash on unit/test-gdbus-client:
==32642== Invalid read of size 4
==32642== at 0x690D0A6: dbus_connection_ref (in
/lib/i386-linux-gnu/libdbus-1.so.3.7.6)
==32642== by 0x804CEDB: message_dispatch (mainloop.c:73)
==32642== by 0x684580E: g_timeout_dispatch (gmain.c:4450)
==32642== by 0x6844A75: g_main_context_dispatch (gmain.c:3065)
==32642== by 0x6844E14: g_main_context_iterate.isra.23 (gmain.c:3712)
==32642== by 0x68452FA: g_main_loop_run (gmain.c:3906)
==32642== by 0x804C7D3: client_connect_disconnect
(test-gdbus-client.c:188)
==32642== by 0x6868DB2: g_test_run_suite_internal (gtestutils.c:2067)
==32642== by 0x6868F8D: g_test_run_suite_internal (gtestutils.c:2138)
==32642== by 0x6869320: g_test_run_suite (gtestutils.c:2189)
==32642== by 0x686936B: g_test_run (gtestutils.c:1508)
==32642== by 0x696D4D2: (below main) (libc-start.c:226)
==32642== Address 0x709c6e4 is 140 bytes inside a block of size 144
free'd
==32642== at 0x67E806C: free (in
/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==32642== by 0x692D62E: dbus_free (in
/lib/i386-linux-gnu/libdbus-1.so.3.7.6)
==32642== by 0x690E1C2: ??? (in
/lib/i386-linux-gnu/libdbus-1.so.3.7.6)
==32642== by 0x804AAEC: destroy_context (test-gdbus-client.c:104)
==32642== by 0x6868DB2: g_test_run_suite_internal (gtestutils.c:2067)
==32642== by 0x6868F8D: g_test_run_suite_internal (gtestutils.c:2138)
==32642== by 0x6869320: g_test_run_suite (gtestutils.c:2189)
==32642== by 0x686936B: g_test_run (gtestutils.c:1508)
==32642== by 0x696D4D2: (below main) (libc-start.c:226)
"Warning: if the connection is disconnected or you try to send Unix file
descriptors on a connection that does not support them, the
DBusPendingCall will be set to NULL, so be careful with this."
Check this in g_dbus_send_message_with_reply so that callers don't need
to double check for NULL if g_dbus_send_message_with_reply returned
TRUE.
This also fix crash if passing FD over D-Bus is blocked e.g. by SELinux
policy.
bluetoothd[1894]: profiles/audio/avdtp.c:session_cb()
bluetoothd[1894]: profiles/audio/avdtp.c:avdtp_parse_cmd() Received
SET_CONFIGURATION_CMD
bluetoothd[1894]: profiles/audio/a2dp.c:endpoint_setconf_ind() Source
0x6c5000: Set_Configuration_Ind
bluetoothd[1894]: profiles/audio/avdtp.c:avdtp_ref() 0x6df360: ref=1
bluetoothd[1894]: profiles/audio/a2dp.c:setup_ref() 0x6d32b0: ref=1
process 1894: arguments to dbus_pending_call_set_notify() were incorrect,
assertion "pending != NULL" failed in file dbus-pending-call.c line
636.
This is normally a bug in some application using the D-Bus library.
