summaryrefslogtreecommitdiff
path: root/src/shared/varlink.c
Commit message (Collapse)AuthorAgeFilesLines
* Drop the text argument from assert_not_reached()Zbigniew Jędrzejewski-Szmek2021-08-031-2/+2
| | | | | | | | | | | | | | | | | In general we almost never hit those asserts in production code, so users see them very rarely, if ever. But either way, we just need something that users can pass to the developers. We have quite a few of those asserts, and some have fairly nice messages, but many are like "WTF?" or "???" or "unexpected something". The error that is printed includes the file location, and function name. In almost all functions there's at most one assert, so the function name alone is enough to identify the failure for a developer. So we don't get much extra from the message, and we might just as well drop them. Dropping them makes our code a tiny bit smaller, and most importantly, improves development experience by making it easy to insert such an assert in the code without thinking how to phrase the argument.
* Use correct `<poll.h>` includeDavid Seifert2021-08-021-1/+1
| | | | * `<sys/poll.h>` is not specified in POSIX
* tree-wide: do not use (void) asprintfZbigniew Jędrzejewski-Szmek2021-07-091-1/+3
| | | | | | | | | | | | | | | asprintf(3) says that the pointer is "undefined" after a failed call. In the current glibc implementation it is just NULL. In principle the call could return a valid pointer with bad contents or something. We have two styles of error handling: in a majority of cases we would check the return value, but sometimes we used (void) and relied on the pointer not being set. In practice both styles should be equivalent, but gcc doesn't like the second one with -Wunused-result. (Though only sometimes. E.g. on my F34 box I don't get the same warnings as in CI, even though the compiler version is very similar and the compilation options are the same…). It's also nice to be consistent in our code base. So let's always use the first style of error checking.
* varlink: remove duplicated "varlink:" prefixZbigniew Jędrzejewski-Szmek2021-06-021-2/+2
| | | | | | | | | | | | | | | | We had: systemd[1]: varlink-36: New incoming message: {"method":"io.systemd.UserDatabase.GetMemberships","parameters":{"userName":"gdm","service":"io.systemd.DynamicUser"},"more":true} systemd[1]: varlink-36: varlink: changing state idle-server → processing-method-more systemd[1]: varlink-36: Sending message: {"error":"io.systemd.UserDatabase.NoRecordFound","parameters":{}} systemd[1]: varlink-36: varlink: changing state processing-method-more → processed-method systemd[1]: varlink-36: varlink: changing state processed-method → idle-server systemd[1]: varlink-36: Got POLLHUP from socket. systemd[1]: varlink-36: varlink: changing state idle-server → pending-disconnect systemd[1]: varlink-36: varlink: changing state pending-disconnect → processing-disconnect systemd[1]: varlink-36: varlink: changing state processing-disconnect → disconnected So let's drop the "varlink:" prefix and use capitalized sentences like in other messages.
* varlink: say "varlink:" instead of "n/a:" when no description is availableZbigniew Jędrzejewski-Szmek2021-06-021-2/+2
| | | | | | | | | | | | For new connections, we log something like this: systemd[1]: n/a: New incoming connection. systemd[1]: n/a: Connections of user 997: 0 (of 1024 max) systemd[1]: varlink-22: varlink: setting state idle-server systemd[1]: varlink-22: New incoming message: ... This "n/a" is not very pretty, and without context it would be hard to even figure out this is a varlink connection.
* alloc-util: simplify GREEDY_REALLOC() logic by relying on malloc_usable_size()Lennart Poettering2021-05-191-11/+8
| | | | | | | | | | | | | | | | | | | | | | | | We recently started making more use of malloc_usable_size() and rely on it (see the string_erase() story). Given that we don't really support sytems where malloc_usable_size() cannot be trusted beyond statistics anyway, let's go fully in and rework GREEDY_REALLOC() on top of it: instead of passing around and maintaining the currenly allocated size everywhere, let's just derive it automatically from malloc_usable_size(). I am mostly after this for the simplicity this brings. It also brings minor efficiency improvements I guess, but things become so much nicer to look at if we can avoid these allocation size variables everywhere. Note that the malloc_usable_size() man page says relying on it wasn't "good programming practice", but I think it does this for reasons that don't apply here: the greedy realloc logic specifically doesn't rely on the returned extra size, beyond the fact that it is equal or larger than what was requested. (This commit was supposed to be a quick patch btw, but apparently we use the greedy realloc stuff quite a bit across the codebase, so this ends up touching *a*lot* of code.)
* Revert "varlink: avoid using dangling ref in varlink_close_unref()"Lennart Poettering2021-05-111-24/+6
| | | | This reverts commit 39ad3f1c092b5dffcbb4b1d12eb9ca407f010a3c.
* varlink: use two local flag variables to silence gcc warningZbigniew Jędrzejewski-Szmek2021-03-311-7/+11
| | | | | | | | | | | | | | | [59/655] Compiling C object src/shared/libsystemd-shared-248.a.p/varlink.c.o ../src/shared/varlink.c: In function ‘varlink_write’: ../src/shared/varlink.c:459:12: warning: ‘n’ may be used uninitialized in this function [-Wmaybe-uninitialized] 459 | if (n < 0) { | ^ ../src/shared/varlink.c: In function ‘varlink_process’: ../src/shared/varlink.c:541:12: warning: ‘n’ may be used uninitialized in this function [-Wmaybe-uninitialized] 541 | if (n < 0) { | ^ ../src/shared/varlink.c:486:17: note: ‘n’ was declared here 486 | ssize_t n; | ^
* varlink: avoid using dangling ref in varlink_close_unref()Zbigniew Jędrzejewski-Szmek2021-03-091-8/+25
| | | | | | | | | | | | | | | | | | | | | | | | Fixes #18025, https://bugzilla.redhat.com/show_bug.cgi?id=1931034. We drop the reference stored in Manager.managed_oom_varlink_request in two code paths: vl_disconnect() which is installed as a disconnect callback, and in manager_varlink_done(). But we also make a disconnect from manager_varlink_done(). So we end up with the following call stack: (gdb) bt 0 vl_disconnect (s=0x112c7b0, link=0xea0070, userdata=0xe9bcc0) at ../src/core/core-varlink.c:414 1 0x00007f1366e9d5ac in varlink_detach_server (v=0xea0070) at ../src/shared/varlink.c:1210 2 0x00007f1366e9d664 in varlink_close (v=0xea0070) at ../src/shared/varlink.c:1228 3 0x00007f1366e9d6b5 in varlink_close_unref (v=0xea0070) at ../src/shared/varlink.c:1240 4 0x0000000000524629 in manager_varlink_done (m=0xe9bcc0) at ../src/core/core-varlink.c:479 5 0x000000000048ef7b in manager_free (m=0xe9bcc0) at ../src/core/manager.c:1357 6 0x000000000042602c in main (argc=5, argv=0x7fff439c43d8) at ../src/core/main.c:2909 When we enter vl_disconnect(), m->managed_oom_varlink_request.n_ref==1. When we exit from vl_discconect(), m->managed_oom_varlink_request==NULL. But varlink_close_unref() has a copy of the pointer in *v. When we continue executing varlink_close_unref(), this pointer is dangling, and the call to varlink_unref() is done with an invalid pointer.
* tree-wide: use -EINVAL for enum invalid valuesZbigniew Jędrzejewski-Szmek2021-02-101-1/+1
| | | | | | | | | As suggested in https://github.com/systemd/systemd/pull/11484#issuecomment-775288617. This does not touch anything exposed in src/systemd. Changing the defines there would be a compatibility break. Note that tests are broken after this commit. They will be fixed in the next one.
* varlink: make 'userdata' pointer inheritance from varlink server to ↵Lennart Poettering2021-01-211-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | connection optional @keszybz's right on https://github.com/systemd/systemd/pull/18248#issuecomment-760798473: swapping out the userdata pointer of a live varlink connection is iffy. Let's fix this by making the userdata inheritance from VarlinkServer object to the Varlink connection object optional: we want it for most cases, but not all, i.e. all those cases where the calls implemented as varlink methods are stateless and can be answered synchronously. For the other cases (i.e. where we want per-connection objects that wrap the asynchronous operation as it goes on) let's not do such inheritance but initialize the userdata pointer only once we have it. THis means the original manager object must be manually retrieved from the VarlinkServer object, which in turn needs to be requested from the Varlink connection object. The userdata inheritance is now controlled by the VARLINK_INHERIT_USERDATA flag passed at VarlinkServer construction. Alternative-to: #18248
* varlink: use hashmap_ensure_putSusant Sahani2021-01-181-4/+3
|
* varlink: add debug loggingZbigniew Jędrzejewski-Szmek2020-12-141-84/+95
| | | | | | | | | | | | When something fails, we need some logs to figure out what happened. This is primarily relevant for connection errors, but in general we want to log about all errors, even if they are relatively unlikely. We want one log on failure, and generally no logs on success. The general idea is to not log in static functions, and to log in the non-static functions. Non-static functions which call other functions may thus log or not log as appropriate to have just one log entry in the end.
* license: LGPL-2.1+ -> LGPL-2.1-or-laterYu Watanabe2020-11-091-1/+1
|
* varlink: add server write states to disconnect checkAnita Zhang2020-10-071-0/+5
| | | | | | | | | | While a server is in the VARLINK_PENDING_METHOD or VARLINK_PENDING_METHOD_MORE states and its write end is disconnected and it gets a POLLHUP, we should disconnect since it can't write anymore. In the case of systemd-oomd disconnecting while pid1 was pending-more, this condition left pid1 in a state where it started throttling from continually getting POLLHUP.
* shared: drop a redundant if statementFrantisek Sumsal2020-09-141-3/+1
|
* varlink: properly allocate connection event sourceLennart Poettering2020-09-041-3/+1
| | | | | | | | Let's make sure we keep a reference to the event source (Note that this code is currently not used, which is why this was never used: in all cases we do not add listener fds after the event is attached, but before. In that case this code is not called.)
* Merge pull request #16925 from cgzones/selinux_create_labelZbigniew Jędrzejewski-Szmek2020-09-011-3/+6
|\ | | | | selinux/core: create several file objects with default SELinux context
| * selinux: create /run/systemd/userdb directory and sockets with default ↵Christian Göttsche2020-09-011-3/+6
| | | | | | | | SELinux context
* | varlink: do not parse invalid messages twiceZbigniew Jędrzejewski-Szmek2020-09-011-3/+9
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Upon reception of a message which fails in json_parse(), we would proceed to parse it again from a deferred callback and hang. Once we have realized that the message is invalid, let's move the pointer in the buffer even if the message is invalid. We don't want to look at this data again. (before) $ build-rawhide/userdbctl --output=json user test.user n/a: varlink: setting state idle-client /run/systemd/userdb/io.systemd.Multiplexer: Sending message: {"method":"io.systemd.UserDatabase.GetUserRecord","parameters":{"userName":"test.user","service":"io.systemd.Multiplexer"}} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state idle-client → awaiting-reply /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state awaiting-reply → pending-disconnect /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state pending-disconnect → disconnected ^C (after) $ n/a: varlink: setting state idle-client /run/systemd/userdb/io.systemd.Multiplexer: Sending message: {"method":"io.systemd.UserDatabase.GetUserRecord","parameters":{"userName":"test.user","service":"io.systemd.Multiplexer"}} /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state idle-client → awaiting-reply /run/systemd/userdb/io.systemd.Multiplexer: New incoming message: {...} /run/systemd/userdb/io.systemd.Multiplexer: Failed to parse JSON: Invalid argument /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state awaiting-reply → pending-disconnect /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state pending-disconnect → processing-disconnect Got lookup error: io.systemd.Disconnected /run/systemd/userdb/io.systemd.Multiplexer: varlink: changing state processing-disconnect → disconnected Failed to find user test.user: Input/output error This should fix #16683 and https://bugs.gentoo.org/735072.
* varlink: add helper for generating errno errorsLennart Poettering2020-08-261-1/+8
|
* tree: wide "the the" and other trivial grammar fixesZbigniew Jędrzejewski-Szmek2020-07-021-1/+1
|
* tree-wide: port to fd_wait_for_event()Lennart Poettering2020-06-101-28/+8
| | | | | | | | Prompted by the discussion on #16110, let's migrate more code to fd_wait_for_event(). This only leaves 7 places where we call into poll()/poll() directly in our entire codebase. (one of which is fd_wait_for_event() itself)
* tree-wide: check POLLNVAL everywhereLennart Poettering2020-06-101-3/+12
| | | | | | | | | | | | | poll() sets POLLNVAL inside of the poll structures if an invalid fd is passed. So far we generally didn't check for that, thus not taking notice of the error. Given that this specific kind of error is generally indication of a programming error, and given that our code is embedded into our projects via NSS or because people link against our library, let's explicitly check for this and convert it to EBADF. (I ran into a busy loop because of this missing check when some of my test code accidentally closed an fd it shouldn't close, so this is a real thing)
* tree-wide: fix spelling errorsFrantisek Sumsal2020-04-211-1/+1
| | | | | | Based on a report from Fossies.org using Codespell. Followup to #15436
* tree-wide: use the return value from sockaddr_un_set_path()Zbigniew Jędrzejewski-Szmek2020-03-021-2/+6
| | | | | | | | | It fully initializes the address structure, so no need for pre-initialization, and also returns the length of the address, so no need to recalculate using SOCKADDR_UN_LEN(). socklen_t is unsigned, so let's not use an int for it. (It doesn't matter, but seems cleaner and more portable to not assume anything about the type.)
* varlink: add ability to register callback for disconnectionsLennart Poettering2020-01-311-2/+21
|
* varlink: add API for determining number of current connectionsLennart Poettering2020-01-311-0/+6
|
* varlink: add varlink_close_unref() helperLennart Poettering2019-12-171-1/+9
|
* json: add flags parameter to json_parse_file(), for parsing "sensitive" dataLennart Poettering2019-12-021-1/+1
| | | | | | | This will call json_variant_sensitive() internally while parsing for each allocated sub-variant. This is better than calling it a posteriori at the end, because partially parsed variants will always be properly erased from memory this way.
* varlink: fix enablement of varlink timeout event sourceLennart Poettering2019-11-221-2/+5
|
* varlink: drop too much whitespaceLennart Poettering2019-11-221-3/+0
|
* varlink: port varlink code over to use getdtablesize() for sizing number of ↵Lennart Poettering2019-11-221-4/+5
| | | | | | | | | concurrent connections Use the official glibc API for determining this parameter. In most other cases in our tree it's better to go directly for RLIMIT_NOFILE since it's semantically what we want, but for this case it appears more appropriate to use the friendlier, shorter, explicit API.
* varlink: move connection fds > fd2Lennart Poettering2019-11-221-0/+4
| | | | | | | | | We want to use this code in NSS modules, and we never know the execution environment we are run in there, hence let's move our fds up to ensure we won't step into dangerous fd territory. This is similar to how we already do it in sd-bus for client connection fds.
* varlink: fix support for more/continues method callsLennart Poettering2019-11-221-17/+80
|
* tree-wide: drop duplicated blank linesYu Watanabe2019-07-151-1/+0
| | | | | | | ``` $ for i in */*.[ch] */*/*.[ch]; do sed -e '/^$/ {N; s/\n$//g}' -i $i; done $ git checkout HEAD -- basic/linux shared/linux ```
* varlink: add varlink server to event loop only if there is oneLennart Poettering2019-07-041-6/+8
|
* varlink: allow using varlink_wait() even with a serverLennart Poettering2019-07-041-1/+0
| | | | | | | | | | | This call can be useful even if a server object is declared. (Originally this was not supported, because a server typically needs to handle multiple connections, and thus a synchronous wait on one would starve the others out. But in some cases it might make sense to have varlink point-to-point connections — i.e. where the server only handles a single connection ever — and there it makes sense to synchronously wait on the one connection).
* shared/varlink: add missing va_end()Zbigniew Jędrzejewski-Szmek2019-05-301-3/+4
| | | | Coverity CID#1401347.
* Revert "varlink: initialize Varlink with 0"Zbigniew Jędrzejewski-Szmek2019-05-301-2/+1
| | | | | | This reverts commit 8688c29b5aece49805a244676cba5bba0196f509, but leaves the reproducer. Structured assignment should be enough to fully initialize the variable and new0 is not necessary.
* shared/varlink: add missing terminator in json stringsZbigniew Jędrzejewski-Szmek2019-05-301-0/+1
| | | | | | | | | Should finally fix oss-fuzz-14688. 8688c29b5aece49805a244676cba5bba0196f509 wasn't enough. The buffer retrieved from memstream has the size that the same as the written data. When we write do write(f, s, strlen(s)), then no terminating NUL is written, and the buffer is not (necessarilly) a proper C string.
* shared/varlink: add missing setting of output_buffer_allocatedZbigniew Jędrzejewski-Szmek2019-05-171-2/+3
| | | | | | | | Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14708, https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14735, https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14725, https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=14720, and probably others.
* varlink: initialize Varlink with 0Yu Watanabe2019-05-161-1/+2
| | | | Closes oss-fuzz#14688.
* journalctl: improve error messagesZbigniew Jędrzejewski-Szmek2019-05-101-3/+8
| | | | Follow-up for #12230.
* Use sd_event_source_disable_unref()Zbigniew Jędrzejewski-Szmek2019-05-101-20/+5
|
* shared: add minimal varlink implementationLennart Poettering2019-05-091-0/+2398
This adds a minimal Varlink (https://varlink.org/) implementation to our tree. Given that we already have a JSON logic it's an easy thing to add. Why bother? We currently have major problems with IPC before dbus-daemon is up, and in all components that dbus-daemon itself makes use of (such as various NSS modules to resolve users as well as the journal which dbus-daemon logs to). Because of that we so far ended up creating various (usually crappy) work-arounds either coming up with secondary IPC systems or sharing data statelessly in /run or similar. Let's clean this up, and instead use a clean, well-defined, broker-less IPC for cases like that. This is a minimal implementation of Varlink, i.e. the most basic logic only. Stuff that's missing is left out on purpose: there's no introspection/validation and there's no name service. It might make sense to add that later, but for now let's only do the minimum buy-in we can get away with. In particular as I'd assume that at least initially we only use this IPC for our internal communication avoiding introspection and the name service should be fine. Specifically, I'd expect that we add IPC interfaces to the following concepts with this scheme: 1. nss-resolve (so that hostname lookups with resolved work before resolved is up) 2. journald (so that IPC calls to journald don't have to go through dbus-daemon thus creating a cyclic dependency between journald and dbus-daemon) 3. nss-systemd (so that dynamic user lookups via PID 1 work sanely even inside of dbus-daemon, because otherwise we'd want to use dbus to run dbus which causes deadlocks) 4. networkd (to make sure one can talk to it in the initrd already, long before dbus is around) And there might be other cases similar to this.
View Lorry Controller Status