test-pending-call-disconnected relies on being run under a session bus.
On master, the TESTS in this directory all get that treatment, but
in dbus-1.10 they do not. This caused test-pending-call-disconnected
to fail in minimal environments like travis-ci where there is no
developer-initiated session bus.
Backport part of commit ec6b220 "name-test: run most C tests directly,
not via run-test.sh" to wrap it in dbus-run-session. This is better
than putting it in run-test.sh because this way, its TAP output is
parsed directly by Automake.
It also has the side benefit of exercising dbus-run-session in the
automated 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=101698
For #100344, we will need a way to store the metadata from the
original method call, and copy them back into arbitrarily many
messages later. This would be easy in GDBus, which has GVariant
as a first-class object. However, libdbus doesn't have an object for
message items, only messages.
We could copy the message's content, but it will carry file descriptors,
which we don't want to copy. Instead, introduce an internal object
representing a message item in a small buffer. It is stored as a variant
(D-Bus type 'v') so that it naturally carries its own type.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
See the doc-comment of the new
dbus_message_iter_abandon_container_if_open() function for details.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
If we run out of memory while calling _dbus_type_writer_recurse()
(which is impossible for most contained types, but can happen for
structs and dict-entries), then the memory we allocated in the call to
_dbus_message_iter_open_signature() will still be allocated, and we
have to free it in order to return to the state of the world prior to
calling open_container().
One might reasonably worry that this change can break callers that use
this (incorrect) pattern:
if (!dbus_message_iter_open_container (outer, ..., inner))
{
dbus_message_iter_abandon_container (outer, inner);
goto fail;
}
/* now we know inner is open, and we must close it later */
However, testing that pattern with _dbus_test_oom_handling()
demonstrates that it already dies with a DBusString assertion failure
even before this commit.
This is all concerningly fragile, and I think the next step should be
to zero out DBusMessageIter instances when they are invalidated, so
that a "double-free" is always detected.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 031aa2ceb3)
Found by source code inspection while trying to debug an unrelated
leak.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
(cherry picked from commit 6b7bdb105b)
This ensures that callers won't accidentally use it for something
in a way that is considered to be programmer error.
In _dbus_message_iter_check(), insert a specific check for this before
dereferencing iter->message, so that we get a nice assertion failure
(potentially non-fatal) instead of a segfault.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
Having opened a container for appending, the container must be closed
exactly once.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
Previously, it was only available under DBUS_ENABLE_EMBEDDED_TESTS,
because the infrastructure to pretend malloc had failed is only
compiled then. However, I'd like to use it in more modular tests, to
avoid test-dbus continuing to grow. To facilitate that, inline a
trivial version of it when DBUS_ENABLE_EMBEDDED_TESTS is disabled:
it just calls the function, once, without doing any strange things to
the malloc interface.
Similarly, amend the stub implementation of
_dbus_get_malloc_blocks_outstanding() so that references to it are
syntactically valid, and move the DBusTestMemoryFunction typedef so
that it can be used with or without DBUS_ENABLE_EMBEDDED_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=101568
If we run out of memory while calling _dbus_type_writer_recurse()
(which is impossible for most contained types, but can happen for
structs and dict-entries), then the memory we allocated in the call to
_dbus_message_iter_open_signature() will still be allocated, and we
have to free it in order to return to the state of the world prior to
calling open_container().
One might reasonably worry that this change can break callers that use
this (incorrect) pattern:
if (!dbus_message_iter_open_container (outer, ..., inner))
{
dbus_message_iter_abandon_container (outer, inner);
goto fail;
}
/* now we know inner is open, and we must close it later */
However, testing that pattern with _dbus_test_oom_handling()
demonstrates that it already dies with a DBusString assertion failure
even before this commit.
This is all concerningly fragile, and I think the next step should be
to zero out DBusMessageIter instances when they are invalidated, so
that a "double-free" is always detected.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
The same assertion appears closer to the top of the function, and there
is no opportunity for it to have become false here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
Found by source code inspection while trying to debug an unrelated
leak.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This was added around 12½ years ago, in a commented-out state, and has
remained commented out ever since. It turns out these test vectors
do pass, although perhaps they didn't at the time.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
The deleted lines used to be a test for _dbus_validate_signature(),
until I deleted that function. We also had a completely separate
test for _dbus_validate_signature_with_reason() which remains present.
Some of the test vectors were tested in both places.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This lets _dbus_warn() and _dbus_warn_check_failed() fall through
to flushing stderr and calling _dbus_abort(), meaning that failed
checks and warnings can result in a core dump as intended.
By renaming the FATAL severity to ERROR, we ensure that any code
contributions that assumed the old semantics will fail to compile.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
All callers should use _dbus_validate_signature_with_reason() directly.
The only remaining callers were this function's own tests.
As a side benefit, this commit removes a TODO pointing out that this
function did not follow normal DBusString conventions, by considering
a length outside the bounds of the DBusString to be an ordinary
lack of validity rather than a fatal programming error.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
As noted in the previous commit, it's a trap.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
This function looks appealing, but it is a trap, particularly in
_dbus_return_val_if_fail() checks. It returns a boolean result, which
cannot distinguish between "failed because we ran out of memory" and
"failed because the string is actually invalid"; but
_dbus_validate_signature_with_reason() allocates memory. Use the
over-complicated version directly, so libdbus can continue to
bend over backwards to support the (possibly mythical) operating systems
that limit memory consumption and do not overcommit, such that malloc()
can genuinely return NULL.
Bug detected by running the DBusVariant unit test (fd.o #101568) under
dbus' failing-malloc() instrumentation.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
If this is reinstated it will need some checks. In particular, it
was using _dbus_check_is_valid_signature() in an unsafe way:
_dbus_check_is_valid_signature() cannot be used in a
_dbus_return_val_if_fail() check because it does not distinguish
between error by the caller, and out-of-memory conditions.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568
By default ${runstatedir} is the same as ${localstatedir}/run, but many
Linux distributions configure it to be /run and mount a tmpfs in that
location. All other factors being equal, it is preferable to use /run
where available because it is guaranteed to be local, whereas traversing
/var might involve automounting a networked filesystem (even though
/var/run itself is very likely to be a tmpfs).
/run or /var/run is currently only used in a few places in dbus, but
I plan to make more use of it during the development of
<https://bugs.freedesktop.org/show_bug.cgi?id=100344>.
The pid file is not part of the API between dbus and other software
(other than distribution init scripts for dbus itself), so we do not
need to keep it strictly compatible; so it is OK to move it.
We do not yet use /run for the system bus socket, because that is
part of the API between D-Bus clients and servers, and has always been
"officially" /var/run/dbus/system_bus_socket.
<https://bugs.freedesktop.org/show_bug.cgi?id=101628> tracks the
possibility of changing that.
Similarly, we do not replace /var/run/console with /run/console, because
that path is part of the API between dbus-daemon and the obsolete PAM
modules pam_console and pam_foreground that used /var/run/console.
<https://bugs.freedesktop.org/show_bug.cgi?id=101629> tracks the possible
future removal of that code path.
In the CMake build system, the equivalent of ${runstatedir} remains
hard-coded to the equivalent of ${localstatedir}/run for simplicity. For
the sort of system-wide installations that would consider redefining
${runstatedir} to /run, the Autotools build system is strongly
recommended: in particular this is what Linux distributions are expected
to use.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101569
The wording and formatting used here is consistent with other
semi-recently-added match keys.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: Wrap BecomeMonitor in <literal> as per Philip's review]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
This is no longer true, and it seems less misleading to raise an
error than to obey the letter of the spec by quietly ignoring calls
from an inappropriate caller.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Now that we're starting to implement methods in more places, it makes
sense to share this code. The Stats interface can already benefit.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
Eavesdropping on unicast messages to other processes is not something
that should be done by processes in containers, or on the system bus
by users other than root or the bus owner. bus/system.conf.in
does not enable eavesdropping, but adding inadvisable configuration
could. This brings it into line with Monitoring.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
When we listen on a tcp: address we should get a connectable tcp:
address, and so on.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
dbus_server_get_address() returns a copy. It isn't clear why.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
These are like unix:tmpdir=/something, except that the resulting
socket is always path-based, never abstract.
This is desirable for two reasons:
* If a Linux container manager wants to expose a path-based socket
into the container, it can do so by bind-mounting it in the
container's filesystem namespace. That cannot work for abstract
sockets because they are not files.
* Conversely, if a Linux container manager does not want to expose
a path-based socket in the container, it can avoid bind-mounting it,
or bind-mount some harmless object like /dev/null over it.
That cannot work for abstract sockets because access to abstract
sockets is part of the network namespace, which is all-or-nothing.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101567
On systemd systems, /etc/machine-id is guaranteed to exist and has
the same format as the D-Bus machine ID. The major D-Bus implementations
read /etc/machine-id if it exists, but some less up-to-date
implementations still only read /var/lib/dbus/machine-id. We can be
nice to those implementations by ensuring /var/lib/dbus/machine-id
is a symlink; this way, the two files can never get out of sync.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101570
