Commit graph

1086 commits

Author SHA1 Message Date
Simon McVittie
39ea37b587 trivial: Remove trailing whitespace from copyright notices
We don't usually mass-remove trailing whitespace from the actual source
code because it would complicate cherry-picking bug fixes to older
branches, but that reasoning doesn't really apply to the comments
containing copyright and licensing notices.

Removing trailing whitespace makes it much easier to move code around:
we have a commit hook that rejects commits containing trailing
whitespace, but that commit hook counts moving a file as a delete + add
pair, so it objects to moving code that contains trailing whitespace.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-17 11:22:39 +00:00
Simon McVittie
83034f38be _dbus_threads_init_debug: Remove
libdbus has been thread-safe by default since 1.7.6 (2013):
dbus_threads_init_default() is called on a just-in-time basis
whenever needed, and _dbus_thread_init_debug() is equivalent to that.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-14 20:06:02 +00:00
Simon McVittie
fa63c16346 .gitignore: Ignore many more generated files
Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-14 13:28:50 +00:00
Simon McVittie
d3fffaa72f .gitignore: Normalize into 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>
2018-12-14 12:53:08 +00:00
Simon McVittie
0c62a9a5f4 .gitignore: Make match patterns more specific
In subdirectories we mostly only want to match at the current level,
not in nested subdirectories.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-14 12:53:08 +00:00
Simon McVittie
c95873e537 .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>
2018-12-14 12:53:08 +00:00
Simon McVittie
0c9f943277 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:

    ./configure SANITIZE_CFLAGS="-fsanitize=address -fsanitize=undefined -fPIE -pie"

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-11 12:47:22 +00:00
Ralf Habacker
ffcaede257 Drop cmake variable DBUS_WIN_FIXME 2018-12-05 15:58:48 +01:00
Ralf Habacker
02bb1c2cfa Avoid memory leaks on running check_shell_service_success_auto_start on Windows 2018-12-05 15:58:48 +01:00
Simon McVittie
152b09ff5a 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
2018-12-03 16:28:13 +00:00
Ralf Habacker
19bc103e5d Enable check_get_connection_unix_process_id test on Windows with CMake
This check is now possible because with merge request
 https://gitlab.freedesktop.org/dbus/dbus/merge_requests/55
the prerequisites are valid.

It was already run if built with Autotools, because DBUS_WIN_FIXME
was only defined in the CMake build system.

[smcv: Add more context regarding Autotools vs. CMake]
Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-12-03 11:31:22 +00:00
Simon McVittie
5659784990 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>
2018-11-29 19:25:57 +00:00
Simon McVittie
94806fb2c7 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>
2018-11-20 19:09:18 +00:00
Simon McVittie
eef153e828 _DBUS_ASSERT_ERROR_XOR_BOOL: Add and use
As suggested by Philip Withnall in dbus!43.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-11-20 12:17:20 +00:00
Simon McVittie
3987f5a4bd 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>
2018-11-20 12:01:12 +00:00
Simon McVittie
269e0548e2 Merge branch 'desktop-file-leak' into 'master'
bus: Fix memory leaks when parsing .service files

See merge request dbus/dbus!42

Reviewed-by: pwithnall
2018-11-20 11:21:32 +00:00
Simon McVittie
2df063be18 BusDesktopFile: Don't leak content if key=value appears before [Section]
BusDesktopFile has a strange convention in which the various parser
helper functions (parse_section_start(), etc.) free the parser on error.
However, this particular error case happens outside the helper functions
and so will leak.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-11-19 11:38:11 +00:00
Simon McVittie
85d07a97e6 bus_container_instance_new: Don't leak empty DBusString object
We already stole its data, but that allocated a new buffer, which we
still need to free.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-11-19 11:35:22 +00:00
Simon McVittie
5accf42aab bus_driver_handle_become_monitor: Don't leak zero-length array of rules
Only privileged users can trigger this leak, so it is not a denial of
service attack.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-11-19 11:35:22 +00:00
Simon McVittie
2908a4da6a embedded tests: Make it easier to run a single test-case
When running tests under "make check" or similar to take advantage
of facilities like AM_TESTS_ENVIRONMENT and AX_VALGRIND_CHECK, it's
more straightforward to set an environment variable than to pass a
command-line option.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://gitlab.freedesktop.org/dbus/dbus/issues/218
2018-11-16 15:16:21 +00:00
Laurent Bigonville
a442601cb2 Use SELINUX_CB_POLICYLOAD instead of AVC_CALLBACK_RESET callback
Use SELINUX_CB_POLICYLOAD instead of AVC_CALLBACK_RESET callback as this
only seems necessary on policy reload and not if the enforcing mode is
changing.

See discussion at https://marc.info/?l=selinux&m=152173501930182&w=2

https://gitlab.freedesktop.org/dbus/dbus/issues/134
2018-11-15 15:59:49 +01:00
Laurent Bigonville
67f7bdf8c2 Stop using avc_init() which is deprecated
Stop using avc_init() and use avc_open() instead. With this commit
dbus-daemon will stop using a thread to monitor the avc netlink and will
poll it instead.

https://gitlab.freedesktop.org/dbus/dbus/issues/134
2018-11-15 15:59:49 +01:00
Simon McVittie
4b46e9cb0f desktop-file: Treat backslashes in section names as literal
The Desktop Entry Specification doesn't give any special meaning to
backslashes in section names: a line "[\n]" starts a section whose
name is the two characters (backslash, n), not a section whose name
is a newline. GKeyFile in GLib matches this interpretation.

In practice, the only section used by dbus-daemon is "D-BUS Service",
only way this could make a difference is if someone had written it
as "D-BUS\sService". According to
https://codesearch.debian.net/search?q=%5C%5BD-BUS%5C%5CsService%5C%5D
there is no instance of that pattern in Debian.

Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-10-19 11:11:55 +00:00
Simon McVittie
99580298f3 desktop-file: Justify implementation of is_valid_section_name()
Signed-off-by: Simon McVittie <smcv@collabora.com>
2018-10-19 11:11:55 +00:00
David King
3ef9e789c1 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
2018-10-19 11:11:55 +00:00
Simon McVittie
2798ecc45f containers: Share code for what happens when we lose a connection
This improves test coverage, because
bus_container_instance_lost_connection() was previously only called
when we failed to set up a connection with a server due to OOM, but
it is now also called (instead of being duplicated) when we are told
to clean up a connection because it has disconnected.

To make sure that connections from containers can never cheat their
way into being treated as uncontained, do not set their
contained_data_slot to NULL.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:43 +01:00
Simon McVittie
ea0275aaea bus_config_parser_check_doctype: Remove, unused
We have never checked the <!DOCTYPE> of busconfig XML since the libxml
parser was removed in 2013, and the libxml parser was broken before
that anyway. The recommended Expat parser (our only parser since 2013)
does not appear to have ever validated this, so now does not seem like
the time to start. Just ignore the <!DOCTYPE> if there is one.

(We never validated this particularly strictly anyway;
<!DOCTYPE busconfig SYSTEM "http://example.com/bees"> would have been
treated as perfectly valid.)

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:38 +01:00
Simon McVittie
10124ae11e BusConfigParser test: Check that all limits are equal, not just one
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:35 +01:00
Simon McVittie
2b8c1b0813 bus_context_get_policy: Remove, unused
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:34 +01:00
Simon McVittie
a3800e2d41 bus_connections_get_context: Remove, unused
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:33 +01:00
Simon McVittie
170adbd6af bus_connections_foreach, bus_connections_foreach_active: Remove
These do not appear in code coverage statistics, and `git grep`
reveals that they are unused.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107739
2018-08-30 17:39:31 +01:00
Simon McVittie
7ae750c4e8 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>
2018-08-23 18:23:21 +01:00
Simon McVittie
bbe7935196 dispatch test: Simplify OOM testing
Instead of having separate test wrappers for the cases that do and
don't take a DBusConnection, we can just pass a NULL DBusConnection
to the one that doesn't.

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
2018-08-02 17:17:07 +01:00
Simon McVittie
46cdc12830 Add and use _dbus_list_clear_full
In gcc 8, -Wall -Wextra includes -Wcast-function-type, which warns
about passing an extra (unwanted) parameter to callbacks. Instead
of using _dbus_list_foreach(), add a function to do what we actually
wanted here.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=107349
Reviewed-by: Thiago Macieira <thiago@kde.org>
2018-08-02 15:26:27 +01:00
Simon McVittie
7512fe4993 containers: Factor out connection_get_instance and make it NULL-safe
When I introduce per-container message filtering, it'll be useful to
be able to get the instance for a connection without worrying about
whether that connection is NULL (representing the dbus-daemon itself,
or an activatable service that has not yet been activated).

Also make it robust against Containers having not been initialized,
for completeness.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105656
2018-06-21 17:42:16 +01:00
Simon McVittie
9a369ee3bd bus: Document arguments of bus_activation_activate_service
It isn't completely obvious that connection is allowed to be NULL 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=105656
2018-06-21 17:42:15 +01:00
Simon McVittie
7389460247 bus_service_list_queued_owners: Don't return a DBusError
This makes it clearer that the only possible error is out-of-memory,
so its use in ListQueuedOwners() is not leaking information to callers
that might not be allowed to know the difference between "doesn't exist"
and "exists but you are not allowed to know that".

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105656
2018-06-21 17:42:12 +01:00
Simon McVittie
f67509cbaf bus_driver_get_owner_of_name: Clarify role of connection
This connection is the one looking at the name, as opposed to the
one that owns the name (if any).

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105656
2018-06-21 17:42:09 +01:00
Simon McVittie
f366c4748a driver: Remove references to an obsolete constant
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105656
2018-06-20 12:12:57 +01:00
Simon McVittie
66246fff2a bus: Clear INVOCATION_ID when carrying out traditional activation
We weren't sure whether this one should be inherited or not, so I
asked on systemd-devel, and Lennart thought it shouldn't.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104641
Reviewed-by: Philip Withnall <withnall@endlessm.com>
2018-03-22 19:25:16 +00:00
Ralf Habacker
7c9f6ca79b bus_connection_get_unix_groups: NULL-check *groups, not groups
groups is never NULL here, but *groups can be NULL on OOM, and that's the
check that was intended.

Coverity ID 265358.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737
Reviewed-by: Simon McVittie <smcv@collabora.com>
2018-03-20 13:27:59 +01:00
Simon McVittie
52594df070 bus driver: Use DBusCredentials to fill credentials structure
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737
2018-03-02 14:52:20 +00:00
Simon McVittie
395269e0bc bus: Get loginfo string bits from DBusCredentials
This saves a couple of _dbus_strdup/dbus_free pairs.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737
2018-03-02 14:52:17 +00:00
Simon McVittie
aaf690e805 bus: Try to get groups directly from credentials, not userdb
If we avoid consulting the userdb, then it's one less chance to
deadlock.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103737
Reviewed-by: Philip Withnall <withnall@endlessm.com>
2018-03-02 14:52:06 +00:00
Simon McVittie
03c0db9ff5 bus: Also tell systemd before we shut down
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104641
2018-03-02 14:49:15 +00:00
Simon McVittie
c937a39e4b bus: Also tell systemd when we're reloading
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104641
2018-03-02 14:49:11 +00:00
Simon McVittie
18a0a8892b bus: Notify systemd when we are ready
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104641
Reviewed-by: Philip Withnall <withnall@endlessm.com>
2018-03-02 14:48:49 +00:00
Simon McVittie
be46707673 bus: Don't pass systemd environment variables to activated services
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104641
Reviewed-by: Philip Withnall <withnall@endlessm.com>
2018-03-02 14:48:24 +00:00
David King
6e42964f5f bus: raise fd limits before dropping privs
Startup ordering was changed in #92832 to ensure that SELinux audit
messages could be sent. As a side effect, the raising of file descriptor
limits was moved to after the dropping of root privileges, resulting in
the limit change always failing.

Move the raise_file_descriptor_limit() call to ensure that it is called
before dropping root privileges.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
Bug-RedHat: https://bugzilla.redhat.com/show_bug.cgi?id=1529044
[smcv: Call raise_file_descriptor_limit() even if !context->user]
Reviewed-by: Simon McVittie <smcv@collabora.com>
2018-02-20 17:40:01 +00:00
Simon McVittie
624e924284 containers: Add a method to ask to be sent the connection instance header
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101899
2018-02-16 15:27:51 +00:00