Simon McVittie [Fri, 14 Dec 2018 12:46:35 +0000 (12:46 +0000)]
.gitignore: Consolidate generic patterns at top level
Patterns in the top-level .gitignore match in all subdirectories, so
there's no need to repeat ourselves quite so much for generic
C, Autotools and gcov patterns.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 14 Dec 2018 12:42:52 +0000 (12:42 +0000)]
Top level .gitignore: Sort anchored patterns in LC_ALL=C sort order
If lists are in a completely arbitrary order, sorting them consistently
means that there is only one correct place to insert a new entry, avoiding
the merge conflicts that would occur if we always append new entries.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 14 Dec 2018 12:42:22 +0000 (12:42 +0000)]
Top level .gitignore: Group anchored and unanchored patterns
The anchored patterns match only in the top directory; the unanchored
patterns match in any subdirectory. While I'm moving the unanchored
patterns around anyway, sort them in lexicographical (LC_ALL=C sort)
order.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 11 Dec 2018 11:52:38 +0000 (11:52 +0000)]
test-privserver: Move helper executable out of name-test/
This means we don't need to distinguish between DBUS_NAME_TEST_EXEC and
DBUS_TEST_EXEC any more, because all test helper executables are in the
same place, both during build and when installed (we don't install
test-privserver since no installed test requires it yet, but in
principle we could).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 13 Dec 2018 13:07:20 +0000 (13:07 +0000)]
test-privserver: Add copyright and licensing information
I've assumed that trivial changes from Scott James Remnant (correcting
some bus names) and Marcus Brinkmann (adding a <config.h> include
tree-wide) are not significant here. All the other potential copyright
holders from the git log are listed, and all have given permission to
relicense their dbus contributions under the MIT/X11 license.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Wed, 29 Aug 2018 19:06:00 +0000 (20:06 +0100)]
build: Add a way to set CFLAGS for AddressSanitizer etc.
We don't want to set these globally via the normal CFLAGS, because if
we did, AddressSanitizer would catch test-segfault deliberately
segfaulting, and "helpfully" turn it into exit status 1, which in turn
makes our test fail because it asserts that the segfault is reported
as a segfault.
A typical use with gcc as compiler, on a reasonably recent Debian,
would be:
Simon McVittie [Mon, 3 Dec 2018 16:04:56 +0000 (16:04 +0000)]
Move _dbus_disable_crash_handling() back into test/ directory
It's only used in test code. We have to put it in its own translation
unit with no non-libc dependencies so that we can compile a copy of it
without AddressSanitizer support, because in a subsequent commit we will
special-case test-segfault to be compiled without using AddressSanitizer,
which would make linking to an AddressSanitizer-instrumented libdbus fail.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 6 Dec 2018 13:49:56 +0000 (13:49 +0000)]
Don't cast user-supplied pointers to DBusBasicValue *
If we cast a user-supplied pointer to DBusBasicValue *, that's like
promising that the pointer is to an object at least as strictly aligned
as a DBusBasicValue. In practice, it often isn't: for example, when
we marshal a dbus_uint32_t, a pointer to it is only guaranteed to be
32-bit-aligned, whereas DBusBasicValue typically requires 64-bit
alignment.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Fri, 7 Dec 2018 18:02:14 +0000 (19:02 +0100)]
run-session: Fix warning 'array subscript 3 is above array bounds of 'char *[3]' on Windows
After the merge request !22 was created, this bug was fixed in !23,
the associated branch was used for local tests, but the fix was not
transferred to !22. After merging !22 into the master branch and
rebasing !23 to the master, this fix was lost.
Ralf Habacker [Thu, 6 Dec 2018 16:08:29 +0000 (17:08 +0100)]
Skip server oom wildcard test on Windows to avoid firewall exception request
After starting the test on Windows 10, whether the local user is an
administrator or not, a dialog box appears with a warning that some
features of the app have been blocked by the firewall and prompts for
credentials for an administrator account while the test continues to
run and pass.
If this request is aborted, the dialog disappears; no restrictions are
visible for the test case. When the test is restarted, the dialog is also
no longer displayed.
In the firewall configuration you can then see that test-server-oom.exe
has been added (but not enabled) for public networks, although no
confirmation has been received from an administrator account.
Ralf Habacker [Wed, 5 Dec 2018 07:30:17 +0000 (08:30 +0100)]
Add new cmake configure option DBUS_USE_WINE
If this variable is set, ctest uses wine to run cross compiled
test applications. Otherwise, they are assumed to run on a native
Windows operating system.
The new cmake variables Z_DRIVE_IF_WINE and TEST_WRAPPER have been
added to support this function.
Ralf Habacker [Wed, 5 Dec 2018 06:56:22 +0000 (07:56 +0100)]
Adds uac manifest to all executable test files to prevent the Windows 740 error when running
The case occurred during test-pending-call-dispatch. To avoid further
applications being affected in the future, the manifest is added to
all test applications.
Windows Error 740 is defined as 'The Requested Operation Requires Elevation'
Simon McVittie [Wed, 5 Dec 2018 13:16:10 +0000 (13:16 +0000)]
CI: Don't install weak dependencies (Recommends)
In particular this avoids installing the TeX toolchain.
However, this also means we don't install dbus, which broke some tests
in minimal containers where dbus wasn't already installed, because the
messagebus user wouldn't have been created. Make sure that user exists,
using the same adduser call as the Debian dbus package.
CMake really wants to find a C++ compiler (even though we only use C++
when compiling for Windows), so explicitly install the default
version of the GNU C++ compiler, g++.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Wed, 5 Dec 2018 12:58:22 +0000 (12:58 +0000)]
sysdeps test: Wait for spawned process to exit
On Windows, the "out" parameter for the process handle isn't set
unless we specify G_SPAWN_FLAGS_DO_NOT_REAP_CHILD. That means we
can't terminate it, and the child process is leaked. When running the
test individually, the leaked process is harmless apart from its
resource cost, but when running under CTest, it holds a file descriptor
open (or something) which causes CTest to not exit.
If we *do* specify G_SPAWN_FLAGS_DO_NOT_REAP_CHILD, we become
responsible for "reaping" the child process by waiting for its exit
status, which is an OS-specific action.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/238
Simon McVittie [Wed, 5 Dec 2018 13:40:46 +0000 (13:40 +0000)]
syslog test: Don't assert that we see plain LF newlines
In at least some build configurations that target Windows, we see CRLF
newlines in the stderr of the subprocess. Leave the newlines unspecified
and let "*" match them.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/243
Ralf Habacker [Wed, 24 Oct 2018 10:03:14 +0000 (12:03 +0200)]
Fix test-shutdown bail out on Windows
test-shutdown expects a GUID in the bus address by default,
which is not available under Windows, because on this platform
an autolaunch address is provided by dbus-run-session and is
not returned by dbus-daemon.
Bug: https://gitlab.freedesktop.org/dbus/dbus/merge_requests/59 Reviewed-by: Simon McVittie <smcv@collabora.com>
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.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 17 Aug 2018 18:50:13 +0000 (19:50 +0100)]
DBusHash: Program a bit more defensively
In particular, the assertions that bucket >= table->buckets and
bucket <= &table->buckets[table->n_buckets - 1] catch the bug fixed
by the previous commit, by ensuring that bucket is somewhere inside
the new array of buckets.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 17 Aug 2018 18:49:36 +0000 (19:49 +0100)]
DBusHash: Recalculate bucket used if the table is rebuilt
Hash buckets are simply entries in an array owned by the hash table,
so every time the hash table's array of buckets is reallocated, we must
invalidate all pointers to buckets and recalculate them to point into
the new array of buckets. This was not always done. Luckily, we appear
to have avoided causing any actual memory corruption like this.
The only place where we reallocate the array of buckets is in
rebuild_table(), which is only called by add_allocated_entry(), which
is only called by add_entry(), which is only called by
find_generic_function() when create_if_not_found is true.
find_generic_function(), in turn, is only called by the
table->find_function() implementations.
The table->find_function() implementations have an optional "out"
parameter which returns a pointer to the hash bucket in which the returned
entry would be found. It is set in find_generic_function() for existing
entries, or in add_allocated_entry() if a new entry is created; after
that it is returned through callers unchanged until the caller of
table->find_function() is reached. The only callers that make use of the
"out" parameter in practice are _dbus_hash_iter_lookup(), to populate
a DBusHashIter, and the _dbus_hash_table_remove_TYPE() family, to pass
it to remove_entry().
We can ignore the _dbus_hash_table_remove_TYPE() family for two
reasons: they call the find function with create_if_not_found set to
FALSE, which never reallocates the hash table, and they do not store
the pointer to the bucket in the long-term. So we only need to consider
_dbus_hash_iter_lookup().
It is documented to be unsafe to add hash entries while a DBusHashIter
is open, and only adding a hash entry can trigger rebuild_table();
so we can assume that if _dbus_hash_iter_lookup() returns a valid
bucket, it remains valid forever.
The remaining case that must be considered is whether reallocation
can occur after setting the "out" parameter for the bucket, but before
returning it to _dbus_hash_iter_lookup(). We can see that it can: we
call rebuild_table() after recalculating the correct bucket. If we do,
and it actually causes a rebuild, then we must recalculate the bucket
accordingly.
Looking at the worst-case impact of this bug, if it is going to cause
any problem, it would only be when _dbus_hash_iter_lookup() is called
with create_if_not_found set true. This makes three uses of the bucket:
it stores it in the DBusHashTableIter, it calculates the next bucket
by finding the offset of the bucket in table->buckets and advancing
by one pointer, and it makes an assertion that should be tautologous,
enforcing that the next bucket corresponds to what it should.
When running under the AddressSanitizer, which makes allocations in
widely spaced regions of memory, on a 32-bit platform, we could (and
indeed do) find that the tautologous assertion fails. The current
bucket returned from the "out" parameter is a pointer into the old
value of table->buckets. If it's far enough before or after the new
table->buckets in the address space, then the offset in next_bucket
could overflow a 32-bit integer, resulting in the assertion no longer
being true.
The next commit will add extra assertions, which reproduce the bug
even without AddressSanitizer.
In production code without assertions, the impact is that
the ->bucket and ->next_bucket members of the DBusHashIter can be
invalid. They are used in _dbus_hash_iter_next() and
_dbus_hash_iter_remove_entry(). However, the only callers of
_dbus_hash_iter_lookup() outside test code are in bus/containers.c,
and neither calls either of those functions, so we dodge that bullet.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 16 Nov 2018 18:53:11 +0000 (18:53 +0000)]
Translate Python-based tests to C
This simplifies bootstrapping: now you don't have to build dbus,
build dbus-python (with GLib), and use dbus-python to test dbus.
It also avoids test failures when using facilities like
AddressSanitizer. When libdbus is built with AddressSanitizer, but the
system copies of Python and dbus-python were not, dbus-python will exit
the Python interpreter on load, because libasan wasn't already
initialized. The simplest way to avoid this is to not use Python:
the scripts are not *that* hard to translate into C.
Both of these tests happen to be conditionally compiled for Unix only.
test_activation_forking() relies on code in TestSuiteForkingEchoService
that calls fork(), which can only work on Unix; meanwhile,
test_system_signals() tests the system bus configuration, which is
only relevant to Unix because we don't support using dbus-daemon as
a privilege boundary on Windows (and in any case D-Bus is not a Windows
OS feature, so the system bus cannot be used to communicate with OS
services like it can on most Linux systems).
This is also a partial solution to
<https://gitlab.freedesktop.org/dbus/dbus/issues/135>, by reducing the
size of name-test/.
For this to work, we need to build the test-service helper executable
even if embedded tests are disabled.
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 [Thu, 29 Nov 2018 19:24:40 +0000 (19:24 +0000)]
bus test: Accept GetConnectionUnixProcessID() success on Windows
Despite its name, which is a historical quirk, this is now a
generic cross-platform process ID on anything with the concept of
numbered processes. It appears it has actually worked on Windows
since dbus 1.7.x.
Bug: https://gitlab.freedesktop.org/dbus/dbus/issues/239 Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 29 Nov 2018 14:24:00 +0000 (14:24 +0000)]
test/dbus-daemon: Mark max-connections-per-user as unimplemented on Windows
The implementation of the max-connections-per-user limit works in terms
of Unix uids, so it doesn't apply on Windows.
This is not a problem in practice, because it only makes sense to limit
connections per user if you have multiple users, and we don't support
the well-known system bus on Windows.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 20 Nov 2018 18:06:34 +0000 (18:06 +0000)]
Don't let dbus-daemon and its subprocesses inherit unnecessary fds
This should avoid test failures under CMake in which the
dbus-daemon inherits an unwanted fd from CMake's test framework, causing
the close-on-exec check before executing activated services to fail.
The dbus-daemon now marks all fds that it inherits, except for its
stdin, stdout and stderr, to be closed on exec. For completeness, the
dbus-daemons run by dbus-run-session and dbus-launch also now inherit
stdin, stdout, stderr and the pipes used to communicate with their
callers, but nothing else.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Tue, 20 Nov 2018 17:32:02 +0000 (17:32 +0000)]
dbus-spawn-unix: If a fd is not close-on-exec, look it up in /proc/self/fd
In operating systems where /proc/self/fd works like it does on Linux
(Linux itself, and FreeBSD with Linux /proc emulation) this will give
us a clue about the fd that was leaked or opened incorrectly.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Tue, 20 Nov 2018 13:06:17 +0000 (14:06 +0100)]
Fix broken meinproc4 based docbook generating by already used xsltproc
Using xsltproc helps to reduce manual editing of xml doc and avoids
cyclic dependency (kdelibs depends on dbus and dbus depends on kdelibs).
It is available on all platforms (in the opposite to xmlto) and supports
freedesktop CI out of the box.
This commit adds docbook-xml and docbook-xsl as new dependency for cmake
and removes obsolate xmlto support, which depends on xsltproc.
Ralf Habacker [Tue, 20 Nov 2018 12:56:39 +0000 (13:56 +0100)]
Cleanup of cmake doc-related targets
There is now a top-level target "doc" that is always built.
Depending on the detected generators it depends on optional
targets like apidoc' and 'devhelp2'.
Simon McVittie [Fri, 16 Nov 2018 17:30:47 +0000 (17:30 +0000)]
BusDesktopFile: Refactor logic to free the parser contents
Now that we have _DBUS_STRING_INIT_INVALID, we can initialize
parser.data to a value that is safe for _dbus_string_free(), which
means we can put all the cleanup through a single code path that
definitely frees everything.
(This is just refactoring, not a correctness fix.)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 19 Nov 2018 13:53:52 +0000 (13:53 +0000)]
containers test: Wait a few more seconds for the socket to be deleted
Previously, we were waiting a few seconds for the dbus-daemon to stop
listening, then trying to connect again and asserting that it failed,
then immediately asserting that the socket had actually been deleted.
However, there is a race here: the dbus-daemon stops listening on the
socket, and then deletes it. If the test client wins the race by
probing to see whether the socket is present after the dbus-daemon
has stopped listening but before the dbus-daemon has deleted it, then
the test will fail.
This intermittently happens on Gitlab-CI, most recently in
<https://gitlab.freedesktop.org/smcv/dbus/-/jobs/45694>.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 31 Aug 2018 18:01:03 +0000 (19:01 +0100)]
tests: Generate random values in unsigned integer space
When we're doing bitwise operations, addition with wraparound and
large left-shifts, it seems safer to use unsigned integers, where
the effect of overflow is well-defined (it wraps around). Signed
integer overflow is undefined behaviour, so compilers are free to
optimize by assuming that it cannot happen.
Detected by the undefined behaviour sanitizer (UBSan).
Signed-off-by: Simon McVittie <smcv@collabora.com>
projects / thirdparty / dbus.git / log
