Simon McVittie [Fri, 10 Dec 2021 15:12:59 +0000 (15:12 +0000)]
internals: Use Standard C offsetof macro if available
clang 13 fails to compile our current implementation with:
.../dbus/dbus-message.c:2070:3: error: variable length array folded to constant array as an extension [-Werror,-Wgnu-folding-constant]
_DBUS_STATIC_ASSERT (_DBUS_ALIGNOF (DBusMessageRealIter) <=
^
.../dbus/dbus-internals.h:460:25: note: expanded from macro '_DBUS_STATIC_ASSERT'
typedef struct { char _assertion[(expr) ? 1 : -1]; } \
This appears to be because the "traditional" definition of
offsetof(), which we're hard-coding here, does not qualify as a constant
expression under C rules due to its use of pointer casts.
Modern compilers like gcc and clang have a built-in implementation
of offsetof that *is* a constant expression.
Calling malloc() after fork is undefined behaviour if the process is
multi-threaded. locks held by a thread on fork() will never be released.
malloc() is usally protected by a lock and can therefore deadlock. glibc
is known not to deadlock in this case.
This commit does not rule out other problems on glibc-systems, but fixes an
issue on musl-libc-systems. Only restricting to async-signal safe functions
between fork() and exec() prevents undefined behaviour for sure. See
signal-safety(7).
Arnout Engelen [Thu, 5 Nov 2020 20:27:02 +0000 (21:27 +0100)]
docs: generate reproducible ids
With this parameter, docbook will generate the same id's when generating the
same docs. This is helpful to get bit-by-bit 'reproducible' output, which
makes auditing the package easier.
William Earley [Sun, 27 Sep 2020 11:39:04 +0000 (12:39 +0100)]
dbus-launch: Replace slashes in DISPLAY if present
dbus-daemon fails to autolaunch with X11 on macOS 10.8+ because XQuartz
(the X11 package for macOS) provides a value for `$DISPLAY` that is
not expected by dbus, in that it contains `/` characters. This is
addressed by replacing the invalid path character `/` with `_`.
William Earley [Sun, 27 Sep 2020 11:24:30 +0000 (12:24 +0100)]
sysdeps: Don't raise RLIMIT_NOFILE beyond OPEN_MAX on macOS
dbus-daemon fails to launch on macOS 10.5 and above because of a breaking
change in setrlimit, in which RLIM_INFINITY is no longer supported
for RLIMIT_NOFILE. Instead we must use OPEN_MAX.
Simon McVittie [Wed, 1 Jul 2020 15:01:38 +0000 (15:01 +0000)]
tests: On Unix, include <netinet/in.h> for IPPROTO_TCP
Otherwise, dbus doesn't compile on FreeBSD if the GLib-based tests
are enabled (which suggests that no FreeBSD user has run those tests
successfully).
We already include <netinet/in.h> in other places with no conditions
or checks other than "is Unix", so apparently it's portable enough that
specifically testing for its presence is not necessary. POSIX requires it
to exist.
Previously, the hash table indexed by uid (or gid) took ownership of the
single reference to the heap-allocated struct, and the hash table
indexed by username (or group name) had a borrowed pointer to the same
struct that exists in the other hash table.
However, this can break down if you have two or more distinct usernames
that share a numeric identifier. This is generally a bad idea, because
the user-space model in such situations does not match the kernel-space
reality, and in particular there is no effective kernel-level security
boundary between such users, but it is sometimes done anyway.
In this case, when the second username is looked up in the userdb, it
overwrites (replaces) the entry in the hash table that is indexed by
uid, freeing the DBusUserInfo. This results in both the key and the
value in the hash table that is indexed by username becoming dangling
pointers (use-after-free), leading to undefined behaviour, which is
certainly not what we want to see when doing access control.
An equivalent situation can occur with groups, in the rare case where
a numeric group ID has two names (although I have not heard of this
being done in practice).
Solve this by reference-counting the data structure. There are up to
three references in practice: one held temporarily while the lookup
function is populating and storing it, one held by the hash table that
is indexed by uid, and one held by the hash table that is indexed by
name.
Simon McVittie [Tue, 30 Jun 2020 18:13:17 +0000 (19:13 +0100)]
userdb: Make lookups return a const pointer
This makes it more obvious that the returned pointer points to a
struct owned by the userdb, which must not be freed or have its
contents modified, and is only valid to dereference until the next
modification to the userdb's underlying hash tables (which in practice
means until the lock is released, because after that we have no
guarantees about what might be going on in another thread).
Simon McVittie [Thu, 16 Apr 2020 13:45:11 +0000 (14:45 +0100)]
sysdeps-unix: On MSG_CTRUNC, close the fds we did receive
MSG_CTRUNC indicates that we have received fewer fds that we should
have done because the buffer was too small, but we were treating it
as though it indicated that we received *no* fds. If we received any,
we still have to make sure we close them, otherwise they will be leaked.
On the system bus, if an attacker can induce us to leak fds in this
way, that's a local denial of service via resource exhaustion.
Each connection that is an active monitor holds a pointer to its own
link in this list, via BusConnectionData.link_in_monitors. We can't
validly free the list while these pointers exist: that would be a
use-after-free, when each connection gets disconnected and tries to
remove itself from the list.
Instead, let each connection remove itself from the list, then assert
that the list has become empty.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/291
Simon McVittie [Thu, 30 May 2019 11:58:28 +0000 (12:58 +0100)]
test: Add basic test coverage for DBUS_COOKIE_SHA1
We don't actually complete successful authentication, because that
would require us to generate a cookie and compute the correct SHA1,
which is difficult to do in a deterministic authentication script.
However, we do assert that dbus#269 (CVE-2019-12749) has been fixed.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 30 May 2019 11:53:03 +0000 (12:53 +0100)]
auth: Reject DBUS_COOKIE_SHA1 for users other than the server owner
The DBUS_COOKIE_SHA1 authentication mechanism aims to prove ownership
of a shared home directory by having the server write a secret "cookie"
into a .dbus-keyrings subdirectory of the desired identity's home
directory with 0700 permissions, and having the client prove that it can
read the cookie. This never actually worked for non-malicious clients in
the case where server uid != client uid (unless the server and client
both have privileges, such as Linux CAP_DAC_OVERRIDE or traditional
Unix uid 0) because an unprivileged server would fail to write out the
cookie, and an unprivileged client would be unable to read the resulting
file owned by the server.
Additionally, since dbus 1.7.10 we have checked that ~/.dbus-keyrings
is owned by the uid of the server (a side-effect of a check added to
harden our use of XDG_RUNTIME_DIR), further ruling out successful use
by a non-malicious client with a uid differing from the server's.
Joe Vennix of Apple Information Security discovered that the
implementation of DBUS_COOKIE_SHA1 was susceptible to a symbolic link
attack: a malicious client with write access to its own home directory
could manipulate a ~/.dbus-keyrings symlink to cause the DBusServer to
read and write in unintended locations. In the worst case this could
result in the DBusServer reusing a cookie that is known to the
malicious client, and treating that cookie as evidence that a subsequent
client connection came from an attacker-chosen uid, allowing
authentication bypass.
This is mitigated by the fact that by default, the well-known system
dbus-daemon (since 2003) and the well-known session dbus-daemon (in
stable releases since dbus 1.10.0 in 2015) only accept the EXTERNAL
authentication mechanism, and as a result will reject DBUS_COOKIE_SHA1
at an early stage, before manipulating cookies. As a result, this
vulnerability only applies to:
* system or session dbus-daemons with non-standard configuration
* third-party dbus-daemon invocations such as at-spi2-core (although
in practice at-spi2-core also only accepts EXTERNAL by default)
* third-party uses of DBusServer such as the one in Upstart
Avoiding symlink attacks in a portable way is difficult, because APIs
like openat() and Linux /proc/self/fd are not universally available.
However, because DBUS_COOKIE_SHA1 already doesn't work in practice for
a non-matching uid, we can solve this vulnerability in an easier way
without regressions, by rejecting it early (before looking at
~/.dbus-keyrings) whenever the requested identity doesn't match the
identity of the process hosting the DBusServer.
Signed-off-by: Simon McVittie <smcv@collabora.com> Closes: https://gitlab.freedesktop.org/dbus/dbus/issues/269 Closes: CVE-2019-12749
Simon McVittie [Mon, 11 Mar 2019 09:03:39 +0000 (09:03 +0000)]
bus: Try to raise soft fd limit to match hard limit
Linux systems have traditionally set the soft limit to 1024 and the hard
limit to 4096. Recent versions of systemd keep the soft fd limit at
1024 to avoid breaking programs that still use select(), but raise the
hard limit to 512*1024, while in recent Debian versions a complicated
interaction between components gives a soft limit of 1024 and a hard
limit of 1024*1024. If we can, we might as well elevate our soft limit
to match the hard limit, minimizing the chance that we will run out of
file descriptor slots.
Unlike the previous code to raise the hard and soft limits to at least
65536, we do this even if we don't have privileges: privileges are
unnecessary to raise the soft limit up to the hard limit.
If we *do* have privileges, we also continue to raise the hard and soft
limits to at least 65536 if they weren't already that high, making
it harder to carry out a denial of service attack on the system bus on
systems that use the traditional limit (CVE-2014-7824).
As was previously the case on the system bus, we'll drop the limits back
to our initial limits before we execute a subprocess for traditional
(non-systemd) activation, if enabled.
systemd activation doesn't involve us starting subprocesses at all,
so in both cases activated services will still inherit the same limits
they did previously.
This change also fixes a bug when the hard limit is very large but
the soft limit is not, for example seen as a regression when upgrading
to systemd >= 240 (Debian #928877). In such environments, dbus-daemon
would previously have changed its fd limit to 64K soft/64K hard. Because
this hard limit is less than its original hard limit, it was unable to
restore its original hard limit as intended when carrying out traditional
activation, leaving activated subprocesses with unintended limits (while
logging a warning).
Reviewed-by: Lennart Poettering <lennart@poettering.net>
[smcv: Correct a comment based on Lennart's review, reword commit message] Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 7eacbfece70f16bb54d0f3ac51f87ae398759ef5)
[smcv: Mention that this also fixes Debian #928877]
Clemens Lang [Fri, 8 Feb 2019 14:48:15 +0000 (15:48 +0100)]
cmake: Avoid overwriting PKG_CONFIG_PATH env var
The CMake config file installed by DBus will run in the context of other
projects. Consequently, changing the value of the PKG_CONFIG_DIR,
PKG_CONFIG_PATH or PKG_CONFIG_LIBDIR environment variables will affect
any further calls to pkg-config made by such projects, which can cause
problems.
A common case of this happening are pkg-config files installed in
usr/share/pkgconfig for .pc files that are architecture-independent, as
for example systemd does.
Avoid clobbering the environment variables by saving and restoring their
values. Note that for some of the variables, setting them to an empty
string is different from not setting them at all.
EAGAIN and EWOULDBLOCK are documented to possibly be numerically equal,
for instance in errno(3), and a simple logical OR check will trigger the
-Wlogical-op warning of GCC. The GCC developers consider the warning to
work as-designed in this case:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69602
Avoid such a warning by explicitly checking if the values are identical.
Fixes: https://gitlab.freedesktop.org/dbus/dbus/issues/225 Signed-off-by: David King <dking@redhat.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit a65319134209d39f5eb6e5425ec6a35fad05bcd7)
David King [Wed, 17 Oct 2018 07:33:25 +0000 (08:33 +0100)]
dbus-send: Avoid duplicated-branches warning
Switch the order of the argument checks to avoid the
-Wduplicated-branches warning.
Signed-off-by: David King <dking@redhat.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit c0bf0d185d72e20e70da9a98e13f69e19f2a87d5)
David King [Fri, 12 Oct 2018 12:58:43 +0000 (13:58 +0100)]
is_valid_section_name: Fix logical expression
Group names in desktop files may contain all ASCII characters, except
control characters and '[' and ']'. Rather than accepting all values,
thanks to a logical operator confusion found by GCC warning
-Wlogical-op, instead explicitly reject the invalid values.
Signed-off-by: David King <dking@redhat.com> Fixes: https://gitlab.freedesktop.org/dbus/dbus/issues/208
(cherry picked from commit 3ef9e789c1b99f420078f4debabd4f5c4fa0a748)
Simon McVittie [Mon, 21 Jan 2019 17:51:07 +0000 (17:51 +0000)]
Adapt to API change in AX_CODE_COVERAGE version 28
AX_CODE_COVERAGE recently changed the way it embedded its Makefile rules
in the output file: instead of using @CODE_COVERAGE_RULES@, users
are now meant to include aminclude_static.am.
The new AX_CODE_COVERAGE is only in the latest autoconf-archive release,
version 2019.01.06, which is inconveniently new, so bundle everything
we need for the moment.
This requires us to stop using the deprecated CODE_COVERAGE_LDFLAGS
(which we still used to support older versions of autoconf-archive)
and replace them with CODE_COVERAGE_LIBS.
Simon McVittie [Mon, 21 Jan 2019 14:28:24 +0000 (14:28 +0000)]
configure.ac: Forbid AX_-prefixed patterns more selectively
We want to make autoconf fail early and with a user-comprehensible
message if autoconf-archive isn't installed, rather than generating
a configure script with syntax errors, or a configure script that runs
successfully but doesn't do what we intended.
However, autoconf-archive doesn't actually guarantee not to use
AX_-prefixed shell variable names without m4_pattern_allow'ing them
(unlike Autoconf, Automake, Libtool and pkg-config, which explicitly use
m4_pattern_allow for variables with AC_, AM_, LT_ and PKG_ prefixes), so
it isn't safe to assume that they won't be used. In particular, recent
versions of AX_CHECK_GNU_MAKE appear to be using
$AX_CHECK_GNU_MAKE_HEADLINE as a shell variable.
Instead, specifically forbid the names of the finite list of macros
that we actually use.
Simon McVittie [Mon, 3 Dec 2018 19:44:47 +0000 (19:44 +0000)]
CI: Stop building on Ubuntu 14.04 'trusty'
The version of gcc in trusty is too old for AddressSanitizer, which we
want to be able to start using, and Travis-CI finally supports Ubuntu
16.04 'xenial' now. This lets us remove some workarounds, but we need
to update others.
Simon McVittie [Tue, 16 Oct 2018 14:44:59 +0000 (15:44 +0100)]
CONTRIBUTING.md: Update and rewrite
This file hadn't kept up with reality, and needs updating for Gitlab.
Take the opportunity to rewrite it.
Much of the text, particularly about commit messages, was taken from
Wayland's contributing guide (thanks to Ander Conselvan de Oliveira,
Bryce Harrington, Eric Engestrom, Pekka Paalanen and Daniel Stone).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 17 Aug 2018 14:42:17 +0000 (15:42 +0100)]
activation: Don't leak if delivering activation message is forbidden
This is technically a denial of service because the dbus-daemon will
run out of memory eventually, but it's a very slow and noisy one,
because all the rejected messages are also very likely to have
been logged to the system log.
Detected by AddressSanitizer.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/234 Reviewed-by: pwithnall
Simon McVittie [Tue, 23 Oct 2018 10:43:23 +0000 (11:43 +0100)]
dbus-daemon test: Allow much longer for pending fd timeout
The timeout we're using here is 0.5s (500ms), but the actual time taken
is unbounded, because the OS scheduler might not schedule our process
for an arbitrary length of time after we become runnable.
We previously allowed up to 1 second, but in the CI jobs for dbus!9
and dbus!18 we've seen this take up to 3.4 seconds (presumably
because other tests, or other jobs running on the same shared
infrastructure, starved this process). Allow up to 10 seconds to guard
against spurious failures.
The timeout used in the production system.conf is 150 seconds (2½
minutes), and we're only using the shorter 500ms timeout here to make
the test complete more quickly, so ±10 seconds is relatively
insignificant: the main thing is that it's finite.
Simon McVittie [Mon, 22 Oct 2018 10:45:45 +0000 (11:45 +0100)]
build: Never use poll() on Darwin family (macOS, etc.) or Interix
Doing a runtime check in configure.ac (AC_RUN_IFELSE) has several
disadvantages:
* It doesn't work when cross-compiling. For example, if we build macOS
binaries on a Linux system, we'd assume that poll() works, but in
fact it won't.
* It checks the build system capabilities, but that is not necessarily
appropriate if (for example) a macOS 10.10 user builds binaries that
could be used by macOS 10.12 or macOS 10.9 users.
* It checks for one specific failure mode, but macOS seems to have a
history of various implementation issues in poll().
* If we want it to work in CMake, we have to duplicate it in the CMake
build system.
None of these is a showstopper on its own, but the combination of all
of them makes the current approach to avoiding the broken poll() on
macOS look unreliable. libcurl, a widely-portable library making
extensive use of sockets, specifically doesn't use poll() on Darwin
(macOS, iOS, etc.) or on Interix; let's follow their example here.
See also https://bugzilla.gnome.org/show_bug.cgi?id=302672 and
https://daniel.haxx.se/blog/2016/10/11/poll-on-mac-10-12-is-broken/
for some relevant history.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/232
(cherry picked from commit 0414ea65ca8196e328da09c3a2324d7765fba8c4)
Simon McVittie [Thu, 4 Oct 2018 16:26:42 +0000 (17:26 +0100)]
ci: Mark many Gitlab jobs to be run manually
freedesktop.org Gitlab doesn't currently have enough test runners
available to run all of this every time. For higher-risk changes
(for example those that change the build system) we can run the
complete set through the web UI.
Simon McVittie [Wed, 3 Oct 2018 16:25:43 +0000 (17:25 +0100)]
ci: Add Gitlab-CI configuration
This uses the same shell scripts as Travis-CI, with slightly different
settings. We use Docker containers for all our Gitlab-CI runs, so take
the opportunity to use Debian 9 'stretch' as our baseline, and
relegate Ubuntu 14.04 'trusty' to to a secondary build.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 60933c09e9e891f74f0102fabe22d29a1a7ae5c5)
Simon McVittie [Wed, 3 Oct 2018 16:51:35 +0000 (17:51 +0100)]
ci: Explicitly install cmake
Travis-CI workers have cmake preinstalled, but Gitlab-CI Docker images
typically don't.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 907832e00849ca454322052981dbb122ea537506)
Simon McVittie [Wed, 3 Oct 2018 16:51:49 +0000 (17:51 +0100)]
ci: Teach ci-install.sh to install wine on Debian 9 'stretch'
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=108177 Acked-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 408b222a9fc61327cd7be385b6705f30f0c38802)
AddrPC.Offset is the same size as a pointer, but previously
we printed it as though it was the same size as a long,
which is 32 bits on 64-bit Windows.
Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105662
(cherry picked from commit 577813cf3a89df804efa6d85a1c5415ba12806ec)
Simon McVittie [Thu, 23 Aug 2018 08:01:03 +0000 (09:01 +0100)]
Do not apply __attribute__((__malloc__)) to dbus_realloc()
As noted in GLib commit c879f50f, gcc's interpretation of the malloc
attribute has become more strict over time, which could result in
miscompilation. The new definition is that in addition to assuming
that the returned memory block is newly-allocated, gcc now assumes
that it does not contain any valid pointers. This is OK for
uninitialized or zero-initialized memory returned by dbus_malloc()
or dbus_malloc0(), but not valid for dbus_realloc(), which might be
used for a dynamically-sized array of (structures containing)
valid pointers.
See https://gitlab.gnome.org/GNOME/glib/issues/1465
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107741
Simon McVittie [Tue, 24 Jul 2018 12:21:37 +0000 (13:21 +0100)]
server-unix: Don't leak address of systemd server on success
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107320 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit d98c43c697fbeb104463dcf2da36d0d855bfb367)
Simon McVittie [Tue, 24 Jul 2018 12:18:48 +0000 (13:18 +0100)]
bus: Free address (from --address) when we have finished using it
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107320 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit 7ae750c4e887b2d63f87caaf2162125ec3217fcc)
Simon McVittie [Thu, 12 Jul 2018 18:11:05 +0000 (19:11 +0100)]
validate_body_helper: Bounds-check before validating booleans
Running the "embedded tests" through valgrind revealed that before this
commit, we would have been willing to read up to 3 bytes off the end of
a message if the message is truncated part way through a boolean. Any
practical allocator will round up allocations to the next 32-bit (or
larger) boundary, so in practice this will not leave the memory buffer
(and in particular did not crash during unit testing), but it could read
uninitialized contents.
On little-endian CPUs, an attacker might be able to use this to learn
whether up to 3 bytes of uninitialized memory in the dbus-daemon
were all-zero (their crafted message would be relayed) or not (their
connection would be disconnected for sending an invalid message). On
big-endian CPUs, an attacker might be able to use this to learn whether
up to 3 bytes were all-zeroes (relayed to a cooperating peer), 0-2
bytes of all-zeroes followed by 0x01 (relayed to a cooperating peer),
or something else (disconnected). This is not believed to be exploitable
to leak interesting information.
Fixes: 62e46533 "hardcode dbus_bool_t to 32 bits"
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107332 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Thiago Macieira <thiago@kde.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit e93a775e68daeda5c95984452aee6327e31c17dd)
Simon McVittie [Thu, 12 Jul 2018 12:32:10 +0000 (13:32 +0100)]
nonce: Don't try to rmdir(NULL) on OOM
If re-initializing the string fails, it will be left in a state
where it has a length of 0 and a NULL buffer. That's valid to
"free", but not valid to pass to rmdir().
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
(cherry picked from commit 294e8b0b672c8ffdcb8d9227f114846433659864)
If an implementation fails to listen, and a subsequent implementation
succeeds, then we would have leaked this. Detected by running
tests/loopback.c under valgrind.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107194
(cherry picked from commit b14a4517a82f7e7e4c0b59cb663ebf77563decda)
Simon McVittie [Mon, 23 Jul 2018 17:52:01 +0000 (18:52 +0100)]
sysdeps: Reassure gcc 8 that we are not overflowing struct sockaddr_un
Using strncpy (buffer, str, strlen (str)) is a "code smell" that
might indicate a serious bug (it effectively turns strncpy into
strcpy), and gcc 8 now warns about it. In fact we avoided the bug
here, but it wasn't at all obvious.
We already checked that path_len is less than or equal to
_DBUS_MAX_SUN_PATH_LENGTH, which is 99, chosen to be strictly less
than the POSIX minimum sizeof(sun_path) >= 100, so we couldn't
actually be overflowing the available buffer.
The new static assertion in this commit matches a comment above the
definition of _DBUS_MAX_SUN_PATH_LENGTH: we define
_DBUS_MAX_SUN_PATH_LENGTH to 99, because POSIX says struct
sockaddr_un's sun_path member is at least 100 bytes (including space
for a \0 terminator). dbus will now fail to compile on
platforms that are non-POSIX-compliant in this way, except for Windows.
We zeroed the struct sockaddr_un before writing into it, so stopping
one byte short of the end of sun_path ensures that we get \0
termination.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107350 Reviewed-by: Thiago Macieira <thiago@kde.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
(cherry picked from commit f429631365ba59a1749438af2184cab138a31772)
Simon McVittie [Mon, 4 Jun 2018 15:27:50 +0000 (16:27 +0100)]
test: Skip TCP tests if getaddrinfo doesn't work
For example, this can be the case in bubblewrap or Debian pbuilder after
unsharing the network namespace:
bwrap \
--bind / / \
--dev-bind /dev /dev \
--bind /dev/shm /dev/shm \
--bind /dev/pts /dev/pts \
--unshare-net \
${builddir}/test/test-loopback --tap
...
ok 1 /connect/tcp # SKIP Name resolution does not work here:
getaddrinfo("127.0.0.1", "0", {flags=ADDRCONFIG, family=INET,
socktype=STREAM, protocol=TCP}): Name or service not known
On some systems this can be circumvented by using nss_wrapper from
<https://cwrap.org/nss_wrapper.html>:
but for systems where that does't work, we should be prepared to skip
the affected tests.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106812
(cherry picked from commit f1faafd59bec67d347edb10447c4b9b18193458c)
Simon McVittie [Mon, 4 Jun 2018 15:27:49 +0000 (16:27 +0100)]
server-oom test: Don't assume localhost is resolvable
Pathological autobuilder environments might not list localhost in
/etc/hosts.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106812
(cherry picked from commit 4cfc7de30de7111f589501e5b597063aeb96cf28)
Simon McVittie [Mon, 4 Jun 2018 15:27:49 +0000 (16:27 +0100)]
test: Test the same things with unix: that we do with tcp:
Minimal autobuilder environments don't always have working TCP,
so we may need to skip TCP tests. Make sure we test the equivalent
code paths via Unix sockets in those environments.
One notable exception is test/fdpass.c, which uses TCP as a transport
that is known not to be able to carry Unix fds; this needs to continue
to use TCP.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106812
(cherry picked from commit cb7dd5bfccb2882d0e9951c41040ff6a97bb827d)
Simon McVittie [Mon, 4 Jun 2018 15:27:48 +0000 (16:27 +0100)]
server-oom test: Parse the address instead of going directly to TCP
This expands test coverage, and lets us reuse the test for other
address schemes.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106812
(cherry picked from commit b19c9e2f265878801bc48866a7cc3152ca2ad45e)
projects / thirdparty / dbus.git / log
