summaryrefslogtreecommitdiff
path: root/dbus
Commit message (Collapse)AuthorAgeFilesLines
* sysdeps-unix: Handle errors from getaddrinfo correctlySimon McVittie2018-06-041-5/+65
| | | | | | | | | | | | | | | | | | getaddrinfo and getnameinfo have their own error-handling convention in which the library call returns either 0 or an EAI_* error code unrelated to errno. If the error code is not EAI_SYSTEM, then the value of errno is undefined (in particular it might be carried over from a previous system call or library call). Introduce a new helper function _dbus_error_from_gai() to handle this. The equivalent code paths in Windows appear to be OK: the Windows implementation of getaddrinfo() is documented to return a Winsock error code, which we seem to be handling correctly. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=106395 (cherry picked from commit 60cedd0cfd775c9fcf7260e12af9b2ffeefc2bbe)
* DBusPendingCall: Improve doc-comments around completed flagSimon McVittie2018-02-061-2/+19
| | | | | | | Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> (cherry picked from commit 57a0cf1d14c20765bfc7a36234955b14f3811f2a)
* DBusPendingCall: Only update ->completed under the connection lockManish Narang2018-02-063-7/+20
| | | | | | | | | | | | | | | | | | | | | | | | | If one thread is blocking on a pending call, and another thread is dispatching the connection, then we need them to agree on the value of the completed flag by protecting all accesses with a lock. Reads for this member seem to have the connection lock already, so it's sufficient to make sure that the only write also happens under the connection lock. We already set the completed flag before calling the callback, so it seems OK to stretch it to meaning that some thread has merely *taken responsibility for* calling the callback. The completed flag shares a bitfield with timeout_added, but that flag is protected by the connection lock already. Based on suggestions from Simon McVittie on <https://bugs.freedesktop.org/show_bug.cgi?id=102839>. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839 [smcv: Revert indentation changes; add commit message] Reviewed-by: Simon McVittie <smcv@collabora.com> (cherry picked from commit d3e03eb50eefa5a38d87f274c7de73f36468459c)
* DBusConnection: Pass a pending call around more oftenManish Narang2018-02-061-1/+1
| | | | | | | | | | | | | | | | | | If a pending call is provided, _dbus_connection_do_iteration_unlocked checks whether it has completed or has a reply ready as soon as it acquires the I/O path. If that's the case, then the iteration terminates without trying to carry out I/O, so that the pending call can be dispatched immediately, without blocking until a timeout is reached. This change is believed to be necessary, but not sufficient, to resolve #102839. Based on part of a patch from Michael Searle on <https://bugs.freedesktop.org/show_bug.cgi?id=102839>. Commit message added by Simon McVittie. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102839 Reviewed-by: Simon McVittie <smcv@collabora.com> (cherry picked from commit 30f8a38b3c8f8756744d6b65dd8207302a683acc)
* _dbus_server_new_for_socket: Iterate over arrays as intendedSimon McVittie2017-11-271-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | Commit 0c03b505 was meant to clear all the fds indexed by j in [0, n_fds), which socket_disconnect() can't be allowed to close (because on failure the caller remains responsible for closing them); but instead it closed the one we failed to add to the main loop (fd i), repeatedly. Similarly, it was meant to invalidate all the watches indexed by j in [i, n_fds) (the one we failed to add to the main loop and the ones we didn't try to add to the main loop yet), which socket_disconnect() can't be allowed to see (because it would fail to remove them from the main loop and hit an assertion failure); but instead it invalidated fd i, repeatedly. These happen to be the same thing if you only have one fd, resulting in the test-case passing on an IPv4-only system, but failing on a system with both IPv4 and IPv6. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> (cherry picked from commit c9aa00ce730f9741ab39ff704e46ec33dd4a11ea)
* _dbus_listen_tcp_socket: Don't rely on dbus_realloc setting errnoSimon McVittie2017-11-241-4/+2
| | | | | | | | | | | | | dbus_realloc() doesn't guarantee to set errno (if it did, the only reasonable thing it could set it to would be ENOMEM). In particular, faking OOM conditions doesn't set it. This can cause an assertion failure when OOM tests assert that the only error that can validly occur is DBUS_ERROR_NO_MEMORY. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104 (cherry picked from commit 9ded6907e66b89c3c74620a4485e8f752f092a60)
* _dbus_server_new_for_socket: Properly disconnect during error unwindingSimon McVittie2017-11-243-14/+38
| | | | | | | | | | | | | | | | _dbus_server_finalize_base() asserts that the socket has been disconnected, but in some OOM code paths we would call it without officially disconnecting. Do so. This means we need to be a bit more careful about what is socket_disconnect()'s responsibility to clean up, what is _dbus_server_new_for_socket()'s responsibility, and what is the caller's responsibility. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104 (cherry picked from commit 0c03b505a9718c6d497caffb7d6083371679a852)
* _dbus_server_new_for_socket: Invalidate watches during error unwindingSimon McVittie2017-11-241-0/+1
| | | | | | | | | | We assert that every watch is invalidated before it is freed, but in some OOM code paths this didn't happen. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104 (cherry picked from commit 1ce34beef85a7a0b3c25890837e3a72f8bdac1f0)
* _dbus_accept_with_noncefile: Don't leak nonceSimon McVittie2017-11-071-5/+17
| | | | | | | | | This was always leaked, both on success and on 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=103597 (cherry picked from commit 37d5af203c0fc22c9ae746b2ae4781ff648a792f)
* do_noncefile_create: Avoid freeing uninitialized memory on errorSimon McVittie2017-11-071-0/+6
| | | | | | | | | | We could free all of these without having ever successfully initialized them. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103597 (cherry picked from commit 6d08f5c04e601f16ef4ee2126a90c924b7e26df0)
* do_check_nonce: Don't free uninitialized memory on OOMSimon McVittie2017-11-071-0/+14
| | | | | | | | | | | | If _dbus_string_init() fails, it doesn't guarantee that the string is initialized to anything in particular. Worse, if _dbus_string_init (&buffer) fails, p would never have been initialized at all, due to the use of the short-circuiting || operator. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103597 (cherry picked from commit 0ea0e4b0fddd1109835b8b9f7a8319d59c8d9303)
* build: Don't distribute versioninfo.rc in "make dist" tarballsSimon McVittie2017-10-231-1/+7
| | | | | | | | | | It's generated by configure, so we should not distribute it. Because it's generated by configure, it is automatically cleaned up by "make distclean" via $(CONFIG_CLEAN_FILES). Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103420
* Update versioninfo.rc.inRalf Habacker2017-10-181-11/+18
| | | | | | | | | | | | | | | | - include <windows.h> to be able to use constants - let versioninfo be visible in explorer by adding a "Translation" value - change FILEOS from VOS_NT_WINDOWS32, which was intended for Windows NT, to VOS__WINDOWS32 - stop setting FILEFLAGS 0x20 (VS_FF_SPECIALBUILD), which is not appropriate here because we build the normal version, not a special version - use constants - fix strings Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de> Reviewed-by: Simon McVittie <smcv@collabora.com>
* Windows: Stop manipulating line numbering in versioninfo.rcSimon McVittie2017-10-171-4/+0
| | | | | | | | | | If __LINE__ doesn't work in MSVC's resource compiler, then removing the #line directive altogether seems a simpler fix than redefining __LINE__ to the wrong value. Signed-off-by: Simon McVittie <smcv@collabora.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
* Windows: Simplify compiling versioninfo.rc by using libtool facilitiesSimon McVittie2017-10-091-17/+5
| | | | | | | | | | | | | | | | | | | | libtool has built-in support for Windows resources, and we even enable it in configure.ac. What it doesn't have is a built-in rule for generating Libtool objects using that built-in support, but we can add one. We have to generate Libtool pseudo-objects (.lo) rather than native object files (.o) so that we get both a PIC object for the shared library and a non-PIC object for the static library. This mimics the libtool invocations used for compiling C and C++. Note that $(RC) is typically i686-w64-mingw32-windres, the same as our project-specific variable $(WINDRES) which was previously used here. Signed-off-by: Simon McVittie <smcv@collabora.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Reviewed-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de>
* unix: Condition Linux-specific abstract sockets on __linux__Simon McVittie2017-09-292-7/+7
| | | | | | | | | | | This is nicer for cross-compiling, because AC_RUN_IFELSE can't work there. In practice abstract sockets are supported on Linux since 2.2 (so, all relevant versions), and on no other platform; so it seems futile to keep this complexity. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34905 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* build: Remove unused variablesSimon McVittie2017-09-281-8/+1
| | | | | | | | | | | libdbus isn't localized, so we have no use for libintl. We always link libdbus-1 with -no-undefined, so we have no use for putting that flag in no_undefined on Windows only. export_symbols seems to be left over from before fd.o#83115 was fixed. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* dbus: Actually link versioninfo.o into libdbusSimon McVittie2017-09-281-0/+1
| | | | | | | | | It appears this has been wrong ever since the versioninfo machinery was first added in 2009, and nobody noticed until now. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* dbus: Clarify why we are not just adding the resource file to SOURCESSimon McVittie2017-09-281-1/+6
| | | | | | Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* dbus: Make SUFFIXES more specificSimon McVittie2017-09-281-1/+1
| | | | | | | | | We want this to apply to files ending with ".rc", but not to files ending with just "rc", like .arc or something. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103015 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* sysdeps: Stop pretending _dbus_set_signal_handler is portable to WindowsSimon McVittie2017-09-273-18/+6
| | | | | | | | | | | | None of the things we rely on in POSIX async signal handlers, such as the existence of async-signal-safe write(), are portable to Windows, so the async signal handlers that use this function are #ifdef DBUS_UNIX anyway. Remove the unused stub function from the Windows side, and move the declaration to the Unix-specific header. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103010 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* Make sure non-aborting signal handlers save and restore errnoSimon McVittie2017-09-271-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | If an async signal interrupts some function, we can have this anti-pattern: /* in normal code */ result = some_syscall (); /* fails, e.g. errno = EINVAL */ /* interrupted by async signal handler */ write (...); /* fails, e.g. errno = ENOBUFS */ /* back to normal code */ if (errno == EINVAL) /* problem! it should be but it isn't */ The solution is for signal handlers to save and restore errno. This is unnecessary for signal handlers that can't touch errno (like the one in dbus-launch that just sets a flag), and for signal handlers that never return (like the one in test-utils-glib for timeouts). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=103010 Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* Deprecate the pam_console/pam_foreground flag-file directorySimon McVittie2017-09-251-1/+4
| | | | | | | | | | | | | | This feature is now compile-time conditional, and off by default. pam_console appears to have been in Fedora and Gentoo until 2007. pam_foreground seems to be specific to Debian and Ubuntu, where it was unmaintained since 2008 and removed in 2010. The replacement for both was ConsoleKit, which has itself been superseded by systemd-logind and ConsoleKit2. Signed-off-by: Simon McVittie <smcv@collabora.com> Bug: https://bugs.freedesktop.org/101629 Reviewed-by: Philip Withnall <withnall@endlessm.com>
* unix: Reduce log level for DBUS_SYSTEM_LOG_INFO to LOG_INFOSimon McVittie2017-09-251-1/+1
| | | | | | | | This is a better match for the way we use it in practice. Signed-off-by: Simon McVittie <smcv@debian.org> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102686 Reviewed-by: Philip Withnall <withnall@endlessm.com>
* Merge branch 'dbus-1.10'Simon McVittie2017-08-151-4/+5
|\
| * Fix -Werror=declaration-after-statement build failure on SolarisAlan Coopersmith2017-08-151-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | dbus-sysdeps-unix.c: In function ‘_dbus_read_credentials_socket’: dbus-sysdeps-unix.c:2061:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] adt_session_data_t *adth = NULL; ^ Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=102145 Reviewed-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
| * sysdeps: increase listen() backlog of AF_UNIX sockets to SOMAXCONNLennart Poettering2017-08-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the listen() backlog was set to an arbitrary 30. This means that if dbus-daemon is overloaded only 30 more connections may be queued by the kernel, before connect() fails with EAGAIN. (Note that EAGAIN != EINPROGRESS -- the latter is what is returned if a connection is queued and being processed for asynchronous sockets; EAGAIN in this case is really an error, that cannot be recovered from). Most software simply sets SOMAXCONN as backlog for AF_UNIX sockets, to allow queuing of as many connections as the kernel allows. SOMAXCONN is 128 on Linux, which is not particularly high, but at least higher than 30. This patch changes dbus-daemon to do the same. I noticed this when flooding dbus-daemon with a lot of connections, where it pretty quickly ceased to respond, much earlier than it really should. Note that the backlog has nothing to do with the number of concurrent connections allowed, it simply controls how many queued, but not accept()ed connections there may be on the listening socket. (cherry picked from commit 12bd6e893c91430fdbdf8a27087d4a792b04eef9) Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95264 Bug-Debian: https://bugs.debian.org/872144 Reviewed-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Thiago Macieira <thiago@kde.org>
| * dbus_message_iter_open_container: Don't leak signature on failureSimon McVittie2017-07-051-9/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 031aa2ceb3dfff373e7b398dfc5d020d77262512)
| * dbus_message_iter_append_basic: Don't leak signature if appending fd failsSimon McVittie2017-07-051-3/+9
| | | | | | | | | | | | | | 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 8384e795516066960bb9fcfbfe138f569420edb9)
| * dbus_message_append_args_valist: Don't leak memory on inappropriate typeSimon McVittie2017-07-051-0/+1
| | | | | | | | | | | | | | | | | | | | 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 6b7bdb105b120b3db312de93af94af1bb6a2a474)
| * transport: Don't pile up errors for semicolon-separated componentsSimon McVittie2017-06-271-0/+3
| | | | | | | | | | | | | | | | | | | | | | If we somehow get an autolaunch address with multiple semicolon-separated components, and one of them fails, then we will hit an assertion failure when we try the next 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=101257 (cherry picked from commit ecdcb86bff42d2bb9cac617bf79f0aa3d47676d9)
| * Fix missing dbus_message_unref() in error reply pathShin-ichi MORITA2017-06-271-0/+2
| | | | | | | | | | | | | | | | | | | | The error message was leaked when blocking on a pending call after the connection was disconnected. Reviewed-by: Philip Withnall <withnall@endlessm.com> [smcv: re-word commit message] Reviewed-by: Simon McVittie <smcv@collabora.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101481
* | Implement dbus_clear_connection(), etc.Simon McVittie2017-07-308-0/+138
| | | | | | | | | | | | | | | | | | | | These are inspired by GLib's g_clear_pointer() and g_clear_object(), which in turn is descended from CPython's Py_CLEAR_OBJECT. They should make our code a lot less repetitive. Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com> Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101895
* | build: Clean up DBUS_COOKIE_SHA1 keyringsSimon McVittie2017-07-281-0/+3
| | | | | | | | | | | | | | We use this directory as the temporary home directory. Fixes: 3f377c511301cfb36bfa93fddf1f59ace8580749 Signed-off-by: Simon McVittie <smcv@debian.org>
* | userdb: Respect $HOME for the home directory of our own uidSimon McVittie2017-07-282-1/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This lets cooperating processes with the same value of $HOME interoperate for DBUS_COOKIE_SHA1 by reading and writing $HOME, even if their $HOME differs from the uid's "official" home directory according to getpwuid(). Out of paranoia, we only do this if the uid and the euid are equal, since if they were unequal the correct thing to do would be ambiguous. In particular, Debian autobuilders run as a user whose "official" home directory in /etc/passwd is "/nonexistent", as a mechanism to detect non-deterministic build processes that rely on the contents of the home directory. Until now, this meant we couldn't run dbus' build-time tests, because every test that used DBUS_COOKIE_SHA1 would fail in this environment. In the tests, set HOME as well as DBUS_TEST_HOMEDIR. We keep DBUS_TEST_HOMEDIR too, because Windows doesn't use HOME, only HOMEDRIVE and HOMEPATH. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101960 Bug-Debian: https://bugs.debian.org/630152 Signed-off-by: Simon McVittie <smcv@debian.org> Reviewed-by: Philip Withnall <withnall@endlessm.com>
* | policy: Add max_fds, min_fds qualifiers for send, receive rulesSimon McVittie2017-07-282-0/+17
| | | | | | | | | | | | | | Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101848 Reviewed-by: Thiago Macieira <thiago@kde.org> [smcv: Revert an incorrect comment change] Signed-off-by: Simon McVittie <smcv@collabora.com>
* | DBusMainLoop: ensure all required timeouts are restartedMichal Koutný2017-07-201-3/+0
| | | | | | | | | | | | | | | | | | This is a followup of 529600397bcab47b9bed5da9208c2df05c8b86b4. We can't shortcut the timeouts iteration in order not to miss any timeouts that might require timestamp restart. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=95619 Reviewed-by: Simon McVittie <smcv@collabora.com>
* | message: Add DBusVariant, a way to copy a single message itemSimon McVittie2017-07-052-0/+257
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | DBusMessageIter: Add a function to abandon possibly-zero-filled iteratorsSimon McVittie2017-07-052-1/+142
| | | | | | | | | | | | | | | | | | 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
* | DBusMessageIter: Zero out the iterator on failureSimon McVittie2017-07-051-2/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | DBusMessageIter: Clarify the APISimon McVittie2017-07-052-3/+21
| | | | | | | | | | | | | | | | | | 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
* | internals: Make a minimal _dbus_test_oom_handling() universally availableSimon McVittie2017-07-051-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | dbus_message_iter_open_container: Don't leak signature on failureSimon McVittie2017-07-051-9/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | dbus_message_iter_append_basic: Don't leak signature if appending fd failsSimon McVittie2017-07-051-3/+9
| | | | | | | | | | | | 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
* | _dbus_message_iter_open_signature: Clarify why this is not leakySimon McVittie2017-07-051-0/+3
| | | | | | | | | | | | | | | | | | 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
* | dbus_message_append_args_valist: Don't leak memory on inappropriate typeSimon McVittie2017-07-051-0/+1
| | | | | | | | | | | | | | | | | | 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
* | _dbus_marshal_validate_test: Uncomment commented-out test coverageSimon McVittie2017-07-051-4/+3
| | | | | | | | | | | | | | | | | | | | 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
* | _dbus_marshal_validate_test: Merge two sets of signature validity checksSimon McVittie2017-07-051-53/+9
| | | | | | | | | | | | | | | | | | | | | | 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
* | internals: Decouple logging an error from exiting unsuccessfullySimon McVittie2017-07-054-17/+5
| | | | | | | | | | | | | | | | | | | | | | | | 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
* | Remove now-unused _dbus_validate_signature()Simon McVittie2017-07-043-41/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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