Adrian Szyndela [Tue, 5 May 2015 11:30:30 +0000 (12:30 +0100)]
DBusCounter: add a mutex to protect the refcount and notify function
The overall problem here is that DBusCounter is indirectly linked
to a DBusConnection, but is not actually guaranteed to be protected by
that connection's mutex; and a DBusMessage can carry a reference to the
DBusCounter, resulting in freeing that DBusMessage having an effect on
the DBusCounter.
Making the refcount atomic would not be a sufficient fix, since it would
not protect the notify function: _dbus_counter_notify() could be called
indirectly by dbus_message_unref(), in an arbitrary thread that does not
hold the DBusConnection's lock, at the same time that the holder
of the DBusConnection lock calls _dbus_transport_set_max_message_size().
[smcv: added commit message]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89297 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Adrian Szyndela [Tue, 5 May 2015 11:27:15 +0000 (12:27 +0100)]
extend lock's range in live_messages_notify()
The other code paths that ref or unref a transport are protected by
the DBusConnection's lock. This function already used that lock,
but for a narrower scope than the refcount manipulation.
live_messages_notify() could be triggered by unreffing messages
that originated from the same connection in a different thread.
[smcv: added commit message]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90312 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
unistd.h and sleep() are normally Unix-specific, although mingw also provides them.
The Windows C runtime documents _environ as its equivalent of Unix environ.
https://bugs.freedesktop.org/show_bug.cgi?id=90089 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Always assert that BUS_CONNECTION_DATA() returns non-NULL
Every DBusConnection in the dbus-daemon should have been through
bus_connections_setup_connection(), so we can assert that the
BusConnectionData has been attached to it. Having this assertion
is enough to hint to Coverity that it does not need to worry about
whether this pointer might be NULL.
In regression tests, we do work with a few fake client-side
DBusConnection instances in the same process; but it would be a
serious bug if we mixed those up with the ones processed by
dbus-daemon's real code, so the assertion is still valid.
This patch has been inspired by (and fixes) the following coverity scan issues:
CID 54846: Dereference null return value (NULL_RETURNS).
CID 54854: Dereference null return value (NULL_RETURNS).
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90021 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[smcv: fixed -Wdeclaration-after-statement; more informative commit message] Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
We already skipped processing for DBUS_ERROR_FILE_NOT_FOUND;
but if the error was something else, we would pass the NULL
pointer dir to _dbus_directory_get_next_file(), which dereferences it.
Reported by Coverity: CID 54744: Dereference after null check (FORWARD_NULL)
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90021
[smcv: re-worded commit message] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Wed, 1 Apr 2015 12:07:19 +0000 (13:07 +0100)]
Change syntax of AM_TESTS_ENVIRONMENT to what the Automake docs prefer
On closer inspection of Automake docs, this is how AM_TESTS_ENVIRONMENT
is actually meant to work; the parallel test driver is even less
compatible with the old serial test driver than I'd realised :-(
Also, according to <http://www.unix.com/man-page/POSIX/1posix/export>,
"export FOO=bar" is actually required functionality for POSIX shells,
and is not a bashism. The Autoconf documentation mentions Solaris 10
as an example of somewhere this doesn't work... but at this point
I'd prefer to say "compiling dbus requires a POSIX shell".
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Mon, 2 Mar 2015 11:46:13 +0000 (11:46 +0000)]
Depend on Automake 1.13 so we can use the correct AM_TESTS_ENVIRONMENT
Since Automake 1.13 (released December 2012) the correct way for a
maintainer to specify environment variables has been
AM_TESTS_ENVIRONMENT, with TESTS_ENVIRONMENT reserved for the user.
That doesn't work in older Automake, so drop support for such old
versions.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:55:45 +0000 (17:55 +0000)]
installed-tests: don't set DBUS_TEST_HOME which is misleading
It doesn't do anything - the variable I was thinking of is called
DBUS_TEST_HOMEDIR. Also, if I had spelled it correctly, the tests
would have failed, because libdbus (quite reasonably) won't create
a nonexistent $HOME to write out cookie_sha1 files in ~/.dbus_keyrings.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:53:46 +0000 (17:53 +0000)]
installed-tests: declare that the output is in TAP format
For the ones written using GLib, the output is in TAP format if we
say --tap. For the one not written using GLib, the output is in TAP
and the command-line is ignored.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 26 Feb 2015 17:11:19 +0000 (17:11 +0000)]
tests: provide g_test_skip() emulation for older GLib
We don't hard-depend on a new enough GLib to have g_test_skip();
if our GLib is older, fake it using g_test_message() and degrade to
reporting it as a pass rather than a skip.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89846
Simon McVittie [Thu, 12 Mar 2015 19:03:12 +0000 (19:03 +0000)]
Fix assorted compiler warnings on Windows.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89444 Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
[rh: rebased because a few hunks have already been applied with commit 92c39d1d8a30110c5760bd8d5e695e26a8538d1a]
Simon McVittie [Wed, 4 Mar 2015 12:11:20 +0000 (12:11 +0000)]
dbus-print-message: conditionalize Unix FD handling on DBUS_UNIX
We close() the fd after we have printed it, but close() isn't
standard functionality on Windows. Unix FD-passing is never going
to work on non-Unix platforms anyway.
Simon McVittie [Wed, 4 Mar 2015 12:10:17 +0000 (12:10 +0000)]
dbus-monitor: use _dbus_get_real_time instead of gettimeofday
gettimeofday is implicitly declared (i.e. not in our #include'd header
files) when cross-compiling for on Windows. Now that fd.o#83115
has been fixed, using _dbus functions is not a problem.
Simon McVittie [Thu, 5 Mar 2015 12:32:05 +0000 (12:32 +0000)]
Improve diagnostics when UpdateActivationEnvironment calls are rejected
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88812 Reviewed-by: Colin Walters <walters@verbum.org>
[smcv: rebased to not require the extra code initially on that bug]
We're ignoring the result of this write() to stderr anyway, because
if it failed... what would we do? Write to stderr? That wouldn't work
any better the second time :-)
Simon McVittie [Wed, 4 Mar 2015 11:58:45 +0000 (11:58 +0000)]
Use new _dbus_string_get_length_uint() to avoid another -Wsign-compare
DBusString's length is signed for historical reasons: the right type
would have been size_t or some other unsigned type. We have a *lot*
of callers of _dbus_string_get_length(), so it is not really desirable
to do a flag-day switch; but we know that the length is always in the
range [0, INT_MAX] that is common to int and unsigned int, so we can
safely add an unsigned accessor.
Peter McCurdy [Tue, 3 Mar 2015 19:52:36 +0000 (20:52 +0100)]
Make all time values signed longs instead of a mix of signed and unsigned, to avoid compiler complaints.
check_timeout() tries to use some unsigned long variables to
perform some intermediate calculations, then has to cast
back to signed long. As it happens, there's no real chance
of overflowing a signed long (it'll only happen if the current
time is within 49.7 days of rolling over, at which point you're
already pretty doomed), so we can make the calculations a bit
simpler, and also avoid the mixed-signedness arithmetic we'd
otherwise need to do.
Simon McVittie [Tue, 24 Feb 2015 15:25:34 +0000 (15:25 +0000)]
monitor test: don't block in main context if we already have messages
Functions like become_monitor() sometimes iterate the main context,
which could leave us with unprocessed messages in f->monitored.
We need to drain that queue of unprocessed messages (setting flags
accordingly, which might meet the loop's exit condition or cause
a break) before we are willing to block in the main context again.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89222 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Ralf Habacker [Tue, 17 Feb 2015 07:26:54 +0000 (08:26 +0100)]
dbus-monitor: Keep term 'dest' in --monitor output in sync with related watch expression.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88896 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[smcv: rebase onto differently indented version of previous commit] Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Ralf Habacker [Mon, 23 Feb 2015 21:00:46 +0000 (22:00 +0100)]
dbus-monitor: Add timestamp to --monitor mode.
Use cross platform function _dbus_get_real_time() for fetching current time.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88896 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
[smcv: use %ld to avoid needing casts; reinstate printing the timestamp;
libdbus-1 is sufficient now that fd.o#83115 is fixed; print timestamp for
non-literal dbus-send replies too] Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
Simon McVittie [Fri, 20 Feb 2015 22:14:48 +0000 (22:14 +0000)]
dbus-launch: if autolaunching, use XDG_RUNTIME_DIR/bus if available
This provides backwards-compatible autolaunching behaviour, as long
as dbus-launch inherits the XDG_RUNTIME_DIR (which it presumably did
if it's going to work at all, since it must also have inherited the
DISPLAY). In particular, we go through the motions of starting the
dbus-daemon, so that we can start the "babysitter" process that will
maintain the X11 window to store the bus address.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61301 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
[smcv: decorate _dbus_lookup_user_bus with DBUS_PRIVATE_EXPORT so we
can still call it after fixing fd.o#83115; update cmake to match Autotools]
Simon McVittie [Fri, 20 Feb 2015 22:06:56 +0000 (22:06 +0000)]
dbus-launch: use libdbus to read the UUID
As a side benefit, this means that dbus-launch now understands
/etc/machine-id and not just /var/lib/dbus/machine-id.
Since machine_uuid comes out of libdbus allocated with dbus_malloc,
to avoid having to copy it from malloc-allocated to
dbus_malloc-allocated storage, it makes sense to change it to be
consistently dbus_malloc-allocated (particularly now that Bug #83115
has made use of internal symbols relatively painless). However, I'm
deliberately not changing the allocation model of any other strings
in dbus-launch right now; that's a larger yak-shaving exercise.
Simon McVittie [Wed, 11 Feb 2015 15:47:53 +0000 (15:47 +0000)]
Add dbus-update-activation-environment tool
If OS builders (distributions) have chosen to use the per-user bus,
this provides two possible modes of operation for compatibility with
existing X session startup hooks.
A legacy-free system can just upload DISPLAY, XAUTHORITY and possibly
DBUS_SESSION_BUS_ADDRESS into dbus-daemon's and systemd's activation
environments, similar to
http://cgit.freedesktop.org/systemd/systemd/tree/xorg/50-systemd-user.sh
installed by systemd (but unlike systemctl,
dbus-update-activation-environment works for traditional
D-Bus-activated services, not just for systemd services).
A system where compatibility is required for environment variables
exported by snippets in /etc/X11/xinit/xinitrc.d (in Red Hat derivatives,
Gentoo, etc.) or /etc/X11/Xsession.d (Debian derivatives) can upload
the entire environment of the X session, minus some selected environment
variables which are specific to a login session (notably XDG_SESSION_ID).
In Debian, I plan to put the former in a new dbus-user-session package
that enables a user-session-centric mode of operation for D-Bus,
and the latter in the existing dbus-x11 package, with the intention that
dbus-x11 eventually becomes a tool for change-averse setups or goes
away entirely.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61301 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Mon, 9 Feb 2015 19:02:43 +0000 (19:02 +0000)]
Optionally install systemd user units for a per-user bus
The socket path used here, $XDG_RUNTIME_DIR/bus, does not match
what was used in user-session-units, but is what Lennart recommended
on fd.o #61303, and is also what kdbus will use for its bus proxy.
Installation of these units switches D-Bus to a different model of
the system: instead of considering each login session (approximately,
each password typed in) to be its own session, the user-session model
is that all concurrent logins by the same user form one large session.
This allows the same bus to be shared by a graphical session, cron jobs,
tty/ssh sessions, screen/tmux sessions and so on.
Because this is a different world-view, it is compile-time optional:
OS builders can choose which world their OS will live in. The default
is still the login-session model used in earlier D-Bus releases,
but might change to the user-session model in future. Explicit
configuration is recommended.
In OSs that support both models (either for sysadmin flexibility or as
a transitional measure), the OS builder should enable the user bus
units, but split them off into a dpkg binary package, RPM subpackage etc.;
the sysadmin can choose whether to enable the user-session model by
choosing whether to install that package.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61301 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Mon, 9 Feb 2015 17:44:53 +0000 (17:44 +0000)]
On Unix platforms, try $XDG_RUNTIME_DIR/bus before default address
This is safe to do even on systems where there is a per-login-session
bus: the $XDG_RUNTIME_DIR/bus would just not exist there. This means
that OS builders can enable a per-user-session bus by merely providing
configuration to start it, without needing to rebuild the client library.
Based on a patch by Colin Walters, with these changes:
- factor out the actual XDG_RUNTIME_DIR bit into a function
- set error correctly on OOM
- do not try to use an XDG_RUNTIME_DIR/bus that belongs to a
different uid or is not a socket
- escape the path if it contains inconvenient characters
- coding style adjustments
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61301 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Simon McVittie [Thu, 8 Jan 2015 14:48:59 +0000 (14:48 +0000)]
Add support for unix:runtime=yes as an address mode
This is not used by default, but can be configured by OS builders (or
regression-test environments) if desired.
If used, this listens on $XDG_RUNTIME_DIR/bus, or fails if $XDG_RUNTIME_DIR
is not set. Fallback behaviour is unnecessary, because it is already
possible to use a string of semicolon-separated addresses like
<listen>unix:runtime=yes;unix:tmpdir=/tmp</listen>, resulting in
listening on either $XDG_RUNTIME_DIR/bus or /tmp/something.
We use a non-abstract socket here, because that is desirable for
use with Linux containers: abstract sockets are attached to the
network namespace, whereas non-abstract sockets are part of the
filesystem and can be bind-mounted between domains if necessary.
The major advantage of abstract sockets is that they do not need
cleanup, but the specification of XDG_RUNTIME_DIR guarantees to
provide cleanup anyway.
Based on prior work by Simon McVittie, Colin Walters and Alexander
Larsson.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=61303 Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
projects / thirdparty / dbus.git / log
