summaryrefslogtreecommitdiffstats
path: root/src (follow)
Commit message (Collapse)AuthorAgeFilesLines
* fuzz-systemctl-parse-argv: call static destuctorsZbigniew Jędrzejewski-Szmek2021-02-161-0/+9
| | | | | | With all the preparatory work in previous PRs, we can now call static destructors repeatedly without issue. We need to do it here so that global variables allocated during parsing are properly freed.
* systemctl: use free_and_replace on global variableZbigniew Jędrzejewski-Szmek2021-02-162-4/+5
| | | | | In normal usage we cannot set it multiple times, but from a fuzzer we may. Doing it this way is nicer anyway.
* tree-wide: reset the cleaned-up variable in cleanup functionsZbigniew Jędrzejewski-Szmek2021-02-1631-56/+66
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the cleanup function returns the appropriate type, use that to reset the variable. For other functions (usually the foreign ones which return void), add an explicit value to reset to. This causes a bit of code churn, but I think it might be worth it. In a following patch static destructors will be called from a fuzzer, and this change allows them to be called multiple times. But I think such a change might help with detecting unitialized code reuse too. We hit various bugs like this, and things are more obvious when a pointer has been set to NULL. I was worried whether this change increases text size, but it doesn't seem to: -Dbuildtype=debug: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd* -Dbuildtype=release: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd* I would expect that the compiler would be able to elide the setting of a variable if the variable is never used again. And this seems to be the case: in optimized builds there is no change in size whatsoever. And the change in size in unoptimized build is negligible. Something strange is happening with size of libsystemd: it's bigger in optimized builds. Something to figure out, but unrelated to this patch.
* tree-wide: return NULL from freeing functionsZbigniew Jędrzejewski-Szmek2021-02-1652-165/+154
| | | | | | I started working on this because I wanted to change how DEFINE_TRIVIAL_CLEANUP_FUNC is defined. Even independently of that change, it's nice to make make things more consistent and predictable.
* networkd: make network_config_section_free() inlineZbigniew Jędrzejewski-Szmek2021-02-162-6/+5
|
* resolved: make dns_transaction_gc return a pointerZbigniew Jędrzejewski-Szmek2021-02-162-8/+8
| | | | | | | | | | | _gc() does cleanup if it is possible. So far it returned a bool to signal if it succeeded (false on success). When working on the resolved code I had to look at the definition every time, because the (arguably reversed) calling convention is unobvious. So let's return a pointer (non-NULL: gc has not been done, NULL: gc has been done). This fits nicely with the standard to return a pointer from all free functions obviously.
* networkd-ndisc: drop confused freepp functionZbigniew Jędrzejewski-Szmek2021-02-161-1/+0
| | | | | | | | | The function to cleanup IPv6Token was defined using freep, i.e. the macro generated a freepp function. The correct way would be to do something like #define ipv6_token_free mfree DEFINE_TRIVIAL_CLEANUP_FUNC(IPv6Token *, ipv6_token_free); which would create ipv6_token_freep(). But since the cleanup function is unused, let's just drop it.
* coredump: add typedef for structZbigniew Jędrzejewski-Szmek2021-02-161-9/+8
|
* basic/capability-util: add missing initializationZbigniew Jędrzejewski-Szmek2021-02-161-1/+1
| | | | There was no error, because the pointer is unconditionally set below.
* analyze: use typedefs for structs and inline iterator variable declsZbigniew Jędrzejewski-Szmek2021-02-161-79/+73
|
* fuzz-systemctl-parse-argv: avoid "leak" of bus objectZbigniew Jędrzejewski-Szmek2021-02-161-0/+3
| | | | | | | | Memory sanitizer would report leaked memory from --boot-load-entry=help. Maybe we should disable all bus connections from the fuzzer? It seems not appropriate to communicate with logind. OTOH, in a real fuzzing environment this call should just fail, so maybe that's OK.
* Merge pull request #18571 from bluca/portable_dbus_docZbigniew Jędrzejewski-Szmek2021-02-165-31/+160
|\ | | | | portable: use helpers for DBUS registration and document DBUS interface
| * portabled: use SD_BUS_METHOD_WITH_ARGSLuca Boccassi2021-02-152-21/+129
| |
| * portabled: use service_parse_argv/bus_add_implementationLuca Boccassi2021-02-155-10/+31
| | | | | | | | Remove some boilerplate and allow introspection
* | Merge pull request #18481 from keszybz/rpm-restart-post-transZbigniew Jędrzejewski-Szmek2021-02-1637-1114/+1537
|\ \ | | | | | | Restart units after the rpm transaction
| * | rpm: restart services in %posttransZbigniew Jędrzejewski-Szmek2021-02-153-1/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes a long-standing issue in packaging scriptlets: daemon-reload was moved to the end of the transaction, but restarting services was still straightaway after package installation. https://bugzilla.redhat.com/show_bug.cgi?id=1614751 Note that daemon-reload is called twice. This wouldn't be hardly noticable, except that now a bunch of units (at least in Fedora) generate very verbose warnings about deprecated features. So we get those warnings twice… reload-or-restart --needing-restart is also called twice, but the second call is usually a noop, because the first clears the flag for restarted units. The second call is necessary for the case where we only uninstall packages, and the %transfiletriggerpostun trigger fires, but not the %transfiletriggerin scriptlet. Also note that this assumes that units are marked only for restart if paths under @systemunitdir@ or /etc/systemd/system have been touched. I would prefer make the trigger that does 'restart --needing-restart' fire always, but it seems rpm doesn't have such functionality. (Except as a %transfiletrigger that would trigger on "/*" to catch all transactions, but that seems ineffiecient and ugly.)
| * | rpm: order sysctl/sysusers/tmpfiles execution before package scriptletsZbigniew Jędrzejewski-Szmek2021-02-152-47/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | P>1000000 is *before* "normal" scriptlets, P<1000000 is *after*. I think it makes sense to do stuff like execution of sysctl/sysusers/tmpfiles configuration before package scriptlets. I think that was the intent, but a single digit got dropped ;( Also, let's reorder the scriptlets in the file to match execution order, to make it easier to see what is going on. Most of those may happen in any order, but there are some exceptions: tmpfiles should be after sysusers, udevadm --reload should be after hwdb.
| * | rpm: simplify daemon-reload triggerZbigniew Jędrzejewski-Szmek2021-02-152-42/+8
| | | | | | | | | | | | | | | | | | | | | | | | The trigger was initially written to use %transfiletriggerun instead of %transfiletriggerpostun because the latter would not fire. It turned out to a buffer overread in rpm that since has been long fixed: https://bugzilla.redhat.com/show_bug.cgi?id=1284645 https://github.com/rpm-software-management/rpm/commit/f6521c50f6836374a0f7995f8f393aaf36e178ea
| * | rpm: sync the shell version of triggers.systemd with the lua versionZbigniew Jędrzejewski-Szmek2021-02-152-48/+34
| | | | | | | | | | | | | | | | | | | | | | | | Note that this goes both ways: in particular the lua version had udev scriptlets in the wrong package, fixed in https://src.fedoraproject.org/rpms/systemd/c/3c9433d7cf4afc8d76660402f6c3d9d991596b83. Add missing "|| :" so the scriptlets never fail.
| * | rpm: pull in the alternative trigger implementation in shZbigniew Jędrzejewski-Szmek2021-02-152-2/+117
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | From https://src.fedoraproject.org/rpms/systemd/blob/master/f/triggers.systemd. In 12dde791d519bc80d5cca4ab6f088763cd481015 scriptlets were converted to lua. This is not only faster and cleaner, but also avoids a nasty dependency loop: rpm implements the lua scripting internally, so we don't need a working shell for the scriplets. This is nice and all, but unfortunately ostree wants to capture scriptlets and execute them at a later time and does not support lua. So in Fedora we ended up with a revert back to a shell-based implementation [1]. At the time I hoped this would only be a temporary workaround, but three years later I think it's fair to assume that this will not happen any time soon. But carrying the upstream lua version and the downstream sh version is error prone. So let's import the other version into our tree too so that they can be kept in sync. [1] https://src.fedoraproject.org/rpms/systemd/c/8e6b39457b3e2660793821e0524855226e33e306
| * | Move rpm stuff into a separate src/rpm/ directoryZbigniew Jędrzejewski-Szmek2021-02-154-4/+16
| | | | | | | | | | | | | | | It is only of interest to rpm-based distros, we can move it out of src/core/ which is pretty busy.
| * | systemctl: add "reload-or-restart --marked"Zbigniew Jędrzejewski-Szmek2021-02-153-13/+84
| | | | | | | | | | | | | | | | | | This is almost equivalent to 'busctl call-method org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager EnqueueMarkedJobs', but waits for the jobs to finish.
| * | systemctl: reduce scope of iterator variablesZbigniew Jędrzejewski-Szmek2021-02-1510-71/+50
| | |
| * | core: wrap long linesZbigniew Jędrzejewski-Szmek2021-02-152-51/+97
| | |
| * | core: add EnqueueMarkedJobs method to reload/restart marked unitsZbigniew Jędrzejewski-Szmek2021-02-153-40/+143
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We support two return types for methods that start jobs. EnqueueJob support the full-monty mode with affected jobs. I didn't do this here, since it seems unlikely to be used. In the common case there'd be a huge list of jobs and affected jobs. EnqueueMarkedJobs() just returns a list of jobs that we can wait upon. The name of the method is generic in case we decide to add something other than just reload/restart later on. When errors occur, resource errors are treated as fatal, but for other error types we queue up other jobs, and only return an error at the end. The assumption is that the caller will ignore the result error anyway, so it's better to try to reload/restart as much as possible.
| * | core: allow Markers to be set using set-propertyZbigniew Jędrzejewski-Szmek2021-02-152-1/+59
| | |
| * | core/dbus-unit: reduce scope of iterator variablesZbigniew Jędrzejewski-Szmek2021-02-151-4/+3
| | |
| * | core: add Unit.Markers propertyZbigniew Jędrzejewski-Szmek2021-02-157-2/+116
| | | | | | | | | | | | | | | | | | | | | | | | | | | The property is never set by systemd, only reset after a stop or restart or reload. It may externally be set to mark the unit for a later restart/reload. I wasn't sure whether to configure the property only for the types where this makes sense (Service, Swap, etc). But Restart() method is defined on the unit, and also having this always under the same property name is more convenient.
| * | core: pahole optimization of struct UnitZbigniew Jędrzejewski-Szmek2021-02-121-10/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We had a lone 'bool job_running_timeout_set:1', which generated a hole. Let's move things around a bit. The structure is a tiny bit smaller and has less holes: /* size: 1192, cachelines: 19, members: 149 */ /* sum members: 1175, holes: 3, sum holes: 11 */ /* sum bitfield members: 27 bits, bit holes: 1, sum bit holes: 7 bits */ /* bit_padding: 14 bits */ /* last cacheline: 40 bytes */ /* size: 1184, cachelines: 19, members: 149 */ /* sum members: 1175, holes: 1, sum holes: 4 */ /* sum bitfield members: 27 bits (3 bytes) */ /* bit_padding: 13 bits */ /* last cacheline: 32 bytes */
| * | manager: remove unnecessary conditionalZbigniew Jędrzejewski-Szmek2021-02-121-10/+7
| | |
| * | core: add helper macros for deserializationZbigniew Jędrzejewski-Szmek2021-02-122-138/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A helper function would seem more natural, but there are two reasons why a macro is needed: - many bool fields are bitfields, so we can't take a pointer, and using a macro allows us to avoid taking a pointer. - we have a few diffent types (bool, uint64_t, FreezerState), and we can have type safety without specifying the type by using the macro. This also makes the error messages more informative: they print the exact field identifier that failed, which is more useful for debugging than a description.
| * | core: split out a few funcs into unit-serialize.[ch]Zbigniew Jędrzejewski-Szmek2021-02-1210-808/+835
| | | | | | | | | | | | Just a straightforward move and resulting include file adjustments.
| * | sd-bus: standarize on NULL for empty signature in method callsZbigniew Jędrzejewski-Szmek2021-02-123-48/+47
| | | | | | | | | | | | | | | We would use sometimes "" and sometimes NULL. They are equivalent, so let's use NULL everywhere, except for a two places in tests.
| * | sd-bus: extend sd_bus_message_read_strv() to paths and signaturesZbigniew Jędrzejewski-Szmek2021-02-121-3/+12
| | | | | | | | | | | | | | | | | | | | | It's rather convenient to be able to read all three types with this function. Strictly speaking this change is not fully compatible, in case someone was relying on sd_bus_message_read_strv() returning an error for anything except "as", but I hope nobody was doing that.
* | | journalctl: rotation is not a reason to warn, but certainly noteworthyLennart Poettering2021-02-151-1/+1
| | | | | | | | | | | | | | | | | | Downgrade the phrasing, since it is a bit misleading. Fixes: #18465
* | | Merge pull request #18579 from keszybz/fix-fragment-id-crashLennart Poettering2021-02-152-9/+12
|\ \ \ | | | | | | | | Fix fragment id crash
| * | | basic/unit-file: when loading linked unit files, use link source as ↵Zbigniew Jędrzejewski-Szmek2021-02-142-9/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "fragment path" The general idea is that when a unit file is "linked" (i.e. installed by symlinking from outside of the search paths), the *destination* name is irrelevant. It doesn't even have to be a valid unit name, or to match the type or instance value. The obvious collorary is that we shouldn't look at the symlink destination name to derive the unit name, instance value, or anything else at all. When building the name map, when we find a linked unit (possibly at the end of a series of alias redirects), store the *source* of the final symlink as the fragment path. This has two effects: - we stop looking at the *target* file name to derive unit info, i.e. actually implement the stuff described in the first paragraph. - we load the unit fragment through the symlink. If someone were to remove the symlink, we'll not load the unit. This seems like the right thing. Fixes #18058. Before this change, we were generally quite confused about unit alises for linked units. Fortunately most poeple use the same symlink source and target, so in practice we wouldn't hit this too often. In unit_load_fragment() a comment is added to explain what we're doing there.
* | | | Merge pull request #18605 from poettering/suppress-repeated-stubLennart Poettering2021-02-155-0/+75
|\ \ \ \ | |_|_|/ |/| | | resolved: filter repeated stub queries
| * | | resolved: filter repeated stub queriesLennart Poettering2021-02-155-0/+75
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's suppress repeated stub queries coming in, to minimize resource usage. Many DNS clients are pretty aggressive regarding repeating DNS requests, hence let's find them and suppress the follow-ups should we need more time to fulfill the queries.
| * | | resolved: allow DNS_PACKET_DATA() argument to be constLennart Poettering2021-02-151-1/+1
| | | |
* | | | resolved: don't redundantly switch DNS servers because of transaction failuresLennart Poettering2021-02-157-19/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a transaction fails and we decide to switch DNS servers, don#t do so unconditionally. Check if the current DNS server is still the same as when the transaction was initiated. And if not, do not do anything. That should reduce the number of redundant DNS server switches if many parallel transactions fail simultaneously (which is pretty likely if DNSSEC is on). Fixes: #17040
* | | | resolved: reuse check for link-local IP address lookupsLennart Poettering2021-02-151-17/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Let's reuse accept_link_local_reverse_lookups() at one more place, where we check for the list of link local reverase address domains. Since we don't actually accept the domains here (but rather the opposite, not accept), let's rename the function a bit more generically with accept_ → match_. While we are at it invert the if branches, to make things more easily understandable: filter out the unwatnted stuff and have the "all good" state as main codepath.
* | | | Merge pull request #18604 from poettering/resolved-minor-tweaksZbigniew Jędrzejewski-Szmek2021-02-152-4/+3
|\ \ \ \ | | | | | | | | | | two minor resolved tweaks
| * | | | resolved: allow DNS_PACKET_DATA() argument to be constLennart Poettering2021-02-151-1/+1
| | | | |
| * | | | resolved: move mdns event sources close to the fdsLennart Poettering2021-02-151-3/+2
| |/ / /
* | | | Merge pull request #18593 from keszybz/fuzz-more-systemctl-pathsLuca Boccassi2021-02-151-50/+46
|\ \ \ \ | |/ / / |/| | | Fuzz more systemctl paths
| * | | systemctl: use argv[0] not program_invocation_short_name for arg dispatchZbigniew Jędrzejewski-Szmek2021-02-151-50/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The immediate motivation is to allow fuzz-systemctl-parse-argv to cover also the other code paths. p_i_s_n is not getting set (and it probably shouldn't), so the fuzzer would only cover the paths for ./systemctl, and not ./reboot, etc. Looking at argv[0] instead, which is passed as part of the fuzzer data, fixes that. But I think in general it's more correct to look at argv[0] here: after all we have all the information available through local variables and shouldn't go out of our way to look at a global.
* | | | rlimit-util: log when falling back setting limitPierre Dubouilh2021-02-151-0/+2
| | | |
* | | | Merge pull request #18580 from keszybz/signal-listLennart Poettering2021-02-1529-139/+246
|\ \ \ \ | | | | | | | | | | Add systemctl --signal=list
| * | | | test-parse-argument: add a test for the three parse_*_argument() functionsZbigniew Jędrzejewski-Szmek2021-02-152-0/+65
| | | | | | | | | | | | | | | | | | | | | | | | | This mostly tests the return values and that the xsprintf buffers are big enough.