bus/selinux: Move vsnprintf call to avoid va_list reuse
In log_callback() the same va_list is reused for a call to vsnprintf and
vsyslog. A va_list can't be reused in this manner, such use is undefined
behavior that changes depending on glibc version.
In current glibc versions a segfault can be observed from the callsite at
bus/selinux.c:412. When trying to log a non-auditable event, the segfault
happens in strlen inside vsyslog.
Moving the call to vsnprintf closer to audit_log_user_avc_message (which is
followed by a 'goto out') avoids the reuse and segfault.
Simon McVittie [Fri, 6 Jan 2023 13:49:17 +0000 (13:49 +0000)]
meson: Use -fvisibility=hidden on Unix if supported
This avoids accidentally exporting all of our private symbols as part
of the LIBDBUS_PRIVATE_x.y.z verdef, including ones that don't need to
be visible outside the shared library even when testing internals
(_dbus_watch_set_handler is a good example).
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/437 Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Tue, 3 Jan 2023 13:29:14 +0000 (14:29 +0100)]
Fix data race in multithreaded application
This commit fixes a data race condition discovered by the
gcc thread sanitizer by also locking the associated mutex
when reading the corresponding counter.
Daniel Wagner [Thu, 24 Nov 2022 08:17:45 +0000 (09:17 +0100)]
meson: Introduce message_bus and tools command line option
To make the consume libdbus via Meson's subproject use case more useful,
introduce message_bus and tools command line options which control if
the D-Bus daemon and/or the tools are build. The idea here is that
depending projects are interested only in the library.
The strong recommendation is only to build libdbus as static library:
Dave Jones [Tue, 3 Jan 2023 18:42:51 +0000 (18:42 +0000)]
Permit access to /tmp dbus socket
After 6e48c317 the test-apparmor-activation test fails as it can no
longer access the dbus socket in /tmp. This commit updates the apparmor
profile used within the test
Simon McVittie [Tue, 29 Nov 2022 20:47:21 +0000 (20:47 +0000)]
internals: Statically assert some things we assume about pointers
Like many relatively-low-level codebases, dbus has historically assumed
that data pointers are interchangeable with function pointers (which is
implied by POSIX and also true on Windows, but not guaranteed by ISO C).
Before dbus!335 was merged, we also assumed that size_t is the same
size as a pointer (which is frequently assumed, but not guaranteed by
ISO C, and notably not true on CHERI). dbus!335 is believed to have
removed all uses of that assumption.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 9 Sep 2022 15:21:43 +0000 (16:21 +0100)]
Use 'continue' keyword in preference to 'goto' where possible
In some more complicated loops, we do need to use 'goto' to exit from
an inner loop, or to jump to cleanup or an increment of an iterator
immediately before the next loop iteration. However, in these simple
cases, jumping to a label immediately before the 'while' keyword is
unnecessary: we can use an equivalent 'continue' statement for flow
control.
This makes it easier for maintainers to notice the loops where we are
doing something more complicated, which still use 'goto', and know
that they need to pay more attention in those cases.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Wed, 18 May 2022 13:35:27 +0000 (14:35 +0100)]
dbus-arch-deps: Mark as AFL-2.0 OR GPL-2.0-or-later
This was probably meant to be relicensed from AFL-2.0 to AFL-2.1 at the
same time as the rest of the codebase, but it wasn't. For now, just
convert its documented license status into machine-readable form.
The history of this file seems to be completely Red Hat and Collabora,
so we should be able to relicense it to (AFL-2.1 OR GPL-2.0-or-later)
or even to MIT, but let's start by making the stated license more
obvious.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This permissive license does not appear to be a match for anything
on the SPDX license list, so we need to use the LicenseRef- prefix
for a custom license.
Co-authored-by: Simon McVittie <smcv@collabora.com>
This permissive license does not appear to be a match for anything
on the SPDX license list, so we need to use the LicenseRef- prefix
for a custom license.
It's referred to as GAP (presumably short for "GNU all-permissive") in
https://sources.debian.org/src/libassuan/2.5.5-1/debian/copyright/
so use the same abbreviation here.
Co-authored-by: Simon McVittie <smcv@collabora.com>
Unlike ASan, by default UBSan prints one-line warnings and keeps going
and it makes it impossible for the CI to catch issues automatically when
it runs the unit tests. With this patch applied the CI should be able to
prevent issues like
https://gitlab.freedesktop.org/smcv/dbus-issue413/-/merge_requests/1#note_1549306
from making it into the repository going forward.
Simon McVittie [Mon, 6 Jun 2022 14:04:57 +0000 (15:04 +0100)]
test-shutdown: Apply a permissive license
All significant contributions to this file were from companies or
individuals that gave permission for relicensing to MIT (I have assumed
that trivial changes from Marcus Brinkmann and Kjartan Maraas were too
small to affect copyright status).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 6 Jun 2022 13:55:42 +0000 (14:55 +0100)]
test-ids: Apply a permissive license
All significant contributions to this file were from companies that gave
permission for relicensing to MIT (I have assumed that trivial changes
from Marcus Brinkmann and Ralf Habacker were too small to affect
copyright status).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 6 Jun 2022 13:52:11 +0000 (14:52 +0100)]
test-autolaunch: Add a permissive license
The license for this file was never stated, but all contributions to it
have been from Red Hat or Collabora, and representatives of both
companies gave permission for MIT relicensing in 2007.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 11 Oct 2022 20:20:50 +0000 (21:20 +0100)]
Autotools: use AX_RECURSIVE_EVAL instead of AS_AC_EXPAND
AX_RECURSIVE_EVAL is maintained as part of autoconf-archive, so we don't
need to bundle our own copy and keep track of its licensing status.
It's very similar to AS_AC_EXPAND, but the arguments are the other way
round and it doesn't automatically AC_SUBST its result.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 11 Oct 2022 20:08:05 +0000 (21:08 +0100)]
m4: Stop shipping our own copy of pkg.m4
The canonical way to find these macro is to run autogen.sh or autoreconf
while pkg-config (or pkgconf) is installed, which will copy an up-to-date
version of the macros from /usr/share/aclocal. The `make dist` archive
will include a bundled copy of those macros, but the git repository
doesn't need them.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 11 Oct 2022 17:41:00 +0000 (18:41 +0100)]
test-autolaunch-win: Remove redundant check for ERROR message
This seems to have been intended to give a more specific error message
if the method call failed, but it will not have been effective, because
dbus_connection_send_with_reply_and_block() ends with a check for ERROR
messages using dbus_set_error_from_message(). This means that if the
reply was an ERROR message, it will already have been converted into a
DBusError by the time call_method() regains control.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 11 Oct 2022 17:36:00 +0000 (18:36 +0100)]
test-autolaunch-win: Don't overwrite an error with another error
An unrelated branch failed CI with this assertion failure:
26: dbus[6768]: error: arguments to dbus_set_error() were incorrect,
assertion "(error) == NULL || !dbus_error_is_set ((error))" failed in
file ...\dbus-errors.c line 365.
Looking at the test, this seems to be the most likely candidate for
this bug in error handling, which is masking whatever the real cause
for the failure was (we can't tell from here). If
dbus_connection_send_with_reply_and_block() returns NULL, then it should
already have set the error.
Fixing this bug in the error handling will hopefully give us a better
error message for the actual failure if it happens again.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 15 Jul 2022 16:10:57 +0000 (17:10 +0100)]
spec: Mention that the system bus address might actually be in /run
The interoperable address is unix:path=/var/run/dbus/system_bus_socket.
However, in most (perhaps all) current Linux distributions, /var/run
is guaranteed to be a symbolic link to /run, and using the path in /run
has some advantages (particularly if automounters are used).
Implementations that intend to be interoperable are not required to
listen on exactly /var/run/dbus/system_bus_socket, as long as clients
that connect to that socket will work correctly. Similarly, clients
are not required to connect to exactly /var/run/dbus/system_bus_socket,
as long as the overall system (consisting of the client and the OSs
that it supports) ensures that it ends up connecting to the same
well-known system bus that is available at
/var/run/dbus/system_bus_socket.
Because of the Unix conventions for how software installs into a prefix,
building a D-Bus implementation with its default build-time
configuration options will not necessarily result in an interoperable
system bus. The system bus is normally shipped by OS distributors,
who should ensure that they have configured it in a way that is
interoperable.
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/180 Signed-off-by: Simon McVittie <smcv@collabora.com>
Issam E. Maghni [Sat, 19 Jun 2021 01:20:21 +0000 (21:20 -0400)]
build: Put system bus socket in runstatedir by default
This lets OS distributors configure --runstatedir=/run if they want to,
although for interoperability, they should only do this if they can
guarantee that their /run and /var/run are equivalent.
A previous commit adds a warning if we are using the default path on a
system where /run and /var/run are not synoymous, mitigating the
compatibility impact of this change.
For CMake, this requires version 3.9, released in 2017.
For Meson, this is currently controlled by the runtime_dir option,
which defaults to /run if the prefix is /usr. The rationale for this
is that /run is correct for modern Unix systems, and distributors who
switch from Autotools or CMake to Meson need to review all their build
options at that time, which is an ideal opportunity to check that they
are doing the right thing around /run.
Helps: https://gitlab.freedesktop.org/dbus/dbus/-/issues/180 Co-authored-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Wed, 18 May 2022 15:13:36 +0000 (16:13 +0100)]
build: Show a warning if the system bus socket is not interoperable
We would like to start using ${runstatedir}/dbus/system_bus_socket,
so that distributors who make /var/run a symbolic link to /run will
usually get their dbus-daemon listening on /run/dbus/system_bus_socket,
which has some advantages in corner cases, such as when /var is mediated
by an automounter or is unmounted during system shutdown.
Unfortunately, the interoperable path in the D-Bus Specification is
/var/run/dbus/system_bus_socket for historical reasons (D-Bus is older
than /run), and older versions of Slackware are known to have had /run
and /var/run as distinct directories. Do a check during configuration
to catch systems configured like this and show a warning.
When cross-compiling, this assumes that the system where dbus is built
(the build system in Autotools/Meson, or the "host" in CMake terminology)
has its /var/run and /run set up in a way that is compatible with the
system where dbus will run (the host system in Autotools/Meson, or the
"target" in CMake terminology). This is not 100% correct, but seems good
enough for a warning that will hopefully only trigger for misguided OS
distributors.
Signed-off-by: Simon McVittie <smcv@collabora.com>
config-parser: no longer get past the last NULL passed to locate_attributes
Fixes: bc86794f23fa53
Fixes:
```
==302818==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7ffd6ac253c0 at pc 0x000000534d0b bp 0x7ffd6ac24e10 sp 0x7ffd6ac24e08
READ of size 8 at 0x7ffd6ac253c0 thread T0
#0 0x534d0a in locate_attributes /home/vagrant/dbus/build/../bus/config-parser.c:658:16
#1 0x52ea3f in start_busconfig_child /home/vagrant/dbus/build/../bus/config-parser.c:1080:12
#2 0x52cca4 in bus_config_parser_start_element /home/vagrant/dbus/build/../bus/config-parser.c:2039:14
#3 0x52b82b in expat_StartElementHandler /home/vagrant/dbus/build/../bus/config-loader-expat.c:107:8
#4 0x7f2179f2d2bd (/lib64/libexpat.so.1+0xd2bd) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2)
#5 0x7f2179f2aed3 (/lib64/libexpat.so.1+0xaed3) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2)
#6 0x7f2179f2c9ec (/lib64/libexpat.so.1+0xc9ec) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2)
#7 0x7f2179f30a8e in XML_ParseBuffer (/lib64/libexpat.so.1+0x10a8e) (BuildId: 0165eed77c910f6ef2227d21afa9c5c5ed5849c2)
#8 0x52b040 in bus_config_load /home/vagrant/dbus/build/../bus/config-loader-expat.c:259:9
#9 0x523c8a in bus_context_new /home/vagrant/dbus/build/../bus/bus.c:828:12
#10 0x521056 in main /home/vagrant/dbus/build/../bus/main.c:716:13
#11 0x7f2179a2954f in __libc_start_call_main (/lib64/libc.so.6+0x2954f) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe)
#12 0x7f2179a29608 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x29608) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe)
#13 0x42a914 in _start (/home/vagrant/dbus/build/bus/dbus-daemon+0x42a914) (BuildId: df5369f85137975aff9bd398ae859706cc3c52ff)
Address 0x7ffd6ac253c0 is located in stack of thread T0 at offset 0 in frame
#0 0x52cfaf in start_busconfig_child /home/vagrant/dbus/build/../bus/config-parser.c:733
```
Simon McVittie [Tue, 13 Sep 2022 15:02:39 +0000 (16:02 +0100)]
dbus-marshal-basic: Fix an incorrect comment
We have 16-bit types with 2-byte alignment, but this comment claimed
we only have 1-, 4- or 8-byte alignment. The actual implementation is
fine, and correctly reports 2-byte alignment for the 16-bit types.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Jordan Williams [Thu, 6 Oct 2022 13:56:36 +0000 (08:56 -0500)]
Check for more symbols before defining HAVE_CLOCK_MONOTONIC in Meson
MacOS has limited support for monotonic clock in its pthread implementation.
When building with Meson, `HAVE_MONOTONIC_CLOCK` is defined despite this.
This PR checks for the existence of the missing function `pthread_condattr_setclock`.
To match parity with AutoTools, it also checks for `clock_getres`.
Simon McVittie [Fri, 30 Sep 2022 12:46:31 +0000 (13:46 +0100)]
dbus-marshal-byteswap: Byte-swap Unix fd indexes if needed
When a D-Bus message includes attached file descriptors, the body of the
message contains unsigned 32-bit indexes pointing into an out-of-band
array of file descriptors. Some D-Bus APIs like GLib's GDBus refer to
these indexes as "handles" for the associated fds (not to be confused
with a Windows HANDLE, which is a kernel object).
The assertion message removed by this commit is arguably correct up to
a point: fd-passing is only reasonable on a local machine, and no known
operating system allows processes of differing endianness even on a
multi-endian ARM or PowerPC CPU, so it makes little sense for the sender
to specify a byte-order that differs from the byte-order of the recipient.
However, this doesn't account for the fact that a malicious sender
doesn't have to restrict itself to only doing things that make sense.
On a system with untrusted local users, a message sender could crash
the system dbus-daemon (a denial of service) by sending a message in
the opposite endianness that contains handles to file descriptors.
Before this commit, if assertions are enabled, attempting to byteswap
a fd index would cleanly crash the message recipient with an assertion
failure. If assertions are disabled, attempting to byteswap a fd index
would silently do nothing without advancing the pointer p, causing the
message's type and the pointer into its contents to go out of sync, which
can result in a subsequent crash (the crash demonstrated by fuzzing was
a use-after-free, but other failure modes might be possible).
In principle we could resolve this by rejecting wrong-endianness messages
from a local sender, but it's actually simpler and less code to treat
wrong-endianness messages as valid and byteswap them.
Simon McVittie [Mon, 12 Sep 2022 12:28:47 +0000 (13:28 +0100)]
test: Add targeted tests for dbus/dbus#413, dbus/dbus#418
Unlike the message-internals test, these do not rely on extra debug
instrumentation in libdbus, and so can be used for "as-installed"
testing. (However, they do require GLib.)
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/413
Reproduces: https://gitlab.freedesktop.org/dbus/dbus/-/issues/418 Signed-off-by: Simon McVittie <smcv@collabora.com>