summaryrefslogtreecommitdiff
path: root/src/shared/varlink.c
Commit message (Collapse)AuthorAgeFilesLines
* parse-util: make parse_fd() return -EBADFYu Watanabe2023-05-081-3/+0
| | | | | | | | The previous error code -ERANGE is slightly ambiguous, and use more specific one. This also drops unnecessary error handlings. Follow-up for 754d8b9c330150fdb3767491e24975f7dfe2a203 and e652663a043cb80936bb12ad5c87766fc5150c24.
* tree-wide: use parse_fd()David Tardon2023-05-051-5/+5
|
* shared: ignore invalid valink socket fd when deserializingFrantisek Sumsal2023-05-031-1/+3
|
* tree-wide: code spelling fixesFrantisek Sumsal2023-04-201-3/+3
| | | | As reported by Fossies.
* varlink: honour "sensitive" flag of json variant objects all the way into ↵Lennart Poettering2023-04-121-5/+15
| | | | | | | | | | | | | the socket Let's honour the flag if it is set, just to be safe. (This only handles the case for the writing side: whenever the client code hands us a json object with the flag set we'll honour it till the it's out of reach for us. This does *not* handle the reading side, which is left for a later patch once needed. We probably should add a per-connection flag that simply globally enables the sensitive logic for all messages coming in on a specific varlink conneciton.)
* varlink: implement file descriptor passingLennart Poettering2023-04-121-22/+426
| | | | | | | | Let's add infrastructure to implement fd passing in varlink, when used over AF_UNIX. This will optionally associate one or more fds with a message sent via varlink and deliver it to the server.
* varlink: add helper that clears the currently processed incoming message ↵Lennart Poettering2023-04-121-6/+13
| | | | | | | | | | | | JSON object Some minor refactoring. This adds a helper call whose only job is to unref the JSON object of the currently processed incoming message. This doesn't make too much sense on its own, given this just replaces one line by another. However, in a later patch when we'll add fd passing we'll extend the function to also destroy associated fds, and then it will start to make more sense.
* varlink: get rid of "reply" fieldLennart Poettering2023-04-121-7/+4
| | | | | | | | | | | | | | | | So far, if we do a synchronous varlink call from the client side via varlink_call(), we'll move the returned json object from "v->current" into "v->reply", and keep it referenced there until the next call. We then return a pointer to it. This ensures that the json object remains valid between two varlink_call() invocations. But the thing is, we don't need a separate field for that, we can just leave the data in "v->current". This means VARLINK_IDLE_CLIENT state will be permitted with and without v->current initialized. Initially, after connection setup it will be set to NULL, but after the first varlink_call() it will be set to the most recent response, pinning it into memory.
* fundamental: rework IN_SET() to require at least three argumentsLennart Poettering2023-01-021-2/+2
| | | | | | | If less than three parameters are passed a simple comparison is the better choice. Lo and behold this found two pretty bad typos.
* tree-wide: use -EBADF for fd initializationZbigniew Jędrzejewski-Szmek2022-12-191-5/+5
| | | | | | | | | | | | | | | | -1 was used everywhere, but -EBADF or -EBADFD started being used in various places. Let's make things consistent in the new style. Note that there are two candidates: EBADF 9 Bad file descriptor EBADFD 77 File descriptor in bad state Since we're initializating the fd, we're just assigning a value that means "no fd yet", so it's just a bad file descriptor, and the first errno fits better. If instead we had a valid file descriptor that became invalid because of some operation or state change, the other errno would fit better. In some places, initialization is dropped if unnecessary.
* treewide: drop "RUN_" from "RUN_WITH_UMASK"Zbigniew Jędrzejewski-Szmek2022-12-131-1/+1
| | | | | | RUN_WITH_UMASK was initially conceived for spawning externals progs with the umask set. But nowadays we use it various syscalls and stuff that doesn't "run" anything, so the "RUN_" prefix has outlived its usefulness.
* varlink: also handle EINTR gracefully when waiting for EIO via ppoll()Lennart Poettering2022-11-221-2/+9
|
* core: serialize/deserialize varlink sockets for pid1Anita Zhang2022-10-141-0/+83
| | | | Fixes #20330
* varlink: refactor adding socket event source to the event loopAnita Zhang2022-10-141-7/+24
|
* varlink: set address field in VarlinkServerSocketAnita Zhang2022-10-111-5/+37
|
* tree-wide: use ASSERT_PTR moreDavid Tardon2022-09-131-12/+6
|
* tree-wide: allow ASCII fallback for → in logsDavid Tardon2022-06-281-1/+3
|
* tree-wide: port various users over to connect_unix_path()Lennart Poettering2022-05-141-9/+15
| | | | Let's make use of our new helper, and thus allow longer paths.
* list: declare iterator of LIST_FOREACH() in the loopYu Watanabe2022-03-191-3/+0
|
* varlink_error_invalid_parameter(...) always returns EINVALVishal Chillara Srinivas2022-03-171-5/+26
| | | | | varlink_error(...) expects a json object as the third parameter. Passing a string variant causes parameter sanitization to fail, and it returns -EINVAL. Pass object variant instead.
* tree-wide: use sd_event_source_disable_unref() where we canLennart Poettering2021-11-091-8/+2
|
* varlink: disconnect varlink link in one more caseLennart Poettering2021-10-221-3/+4
| | | | | | | | | | Previously we'd possibly see POLLHUP on a varlink link, and continue to run epoll on it even though we have nothing to read nor write anymore. Let's fix that, and once we know that there's nothing to write anymore (or we saw a write error already) we'll disconnect after POLLHUP. Fixes: #20062
* varlink: make one more parameter constLennart Poettering2021-10-111-1/+1
|
* util: define initializer for 'struct ucred' that properly invalidates all fieldsLennart Poettering2021-10-111-3/+2
| | | | | i.e. let's make sure to invalid uid/gid to UID_INVAID + GID_INVALID instead of zero.
* 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.)