Like the command-level CommandDescription.argspec, a verb's argspec is now
a nulstr, so that a verb may declare multiple alternative usage lines. The
verbs help table renders one synopsis line per nulstr entry in the
synopsis cell.
All verb argument specifications must gain an explicit NUL suffix. This
is a lot of churn, but I think this needs to be done. For now, there'll
be only one user of the new functionality (i.e. a verb with more than
one argspec), but I think we'll eventually want to use this in other
places too.
This converts systemd-homed, -hostnamed, -importd, -localed, -logind,
-machined, -networkd, -portebled, -resolved, -sysupdated, -timedated,
and -timesyncd.
The service name and description move from service_parse_argv() arguments
into a COMMAND() description that each daemon now defines. The
service_parse_argv() macro captures the calling binary's verb section,
where that COMMAND is found, while the options are still defined and
parsed in libshared, in the new "service" option namespace that each
COMMAND references. (The named namespace makes sure that options defined
by other translation units in libshared can never leak into the
services' parsers.)
The supported features are declared via option group filtering: each
COMMAND lists "Options" plus "Bus introspection" and/or "Runtime scope".
Unsupported options are now rejected by the parser as unknown options
(and are not shown in --help), replacing the runtime "This service does
not support …" errors.
The help output changes slightly: the "Bus introspection" and "Runtime
scope" option groups now get their own headings, and some descriptions
were reworded to the imperative form used elsewhere.
This time, various commands with non-trivial requirements are handled.
So the series is shorter, but has a pattern of feature additions and
then those new features being used. The way that --help output is
formatted is changed a bit in various places. Those changes are
described in individual commits.
22 'static int help()' functions left after this.
We already have io.systemd.service.SetLogLevel. Add a getter so that
CLI tools with `log-level` subcommands can be more easily implemented
with varlink.
Bind this method to all services that already bind io.systemd.service.SetLogLevel.
Almost all programs are simple and have only a single command. When
printing options, let's just use the first COMMAND block found by default.
This means we can drop the explicit program-name argument and reduce
copy&paste boilerplate.
The DBus API already exposes the parsed machine-tags list via the Tags
property but varlink clients needed to parse the raw TAGS= line from
MachineInformationData (which also required implementing tag name
validation if the tags would later be used with SetTags, to avoid
spurious errors).
So, provide the same information that DBus gets (a list of pre-parsed
tags with bad tags gracefully removed) as a new MachineTags field
returned by Describe -- this does mean that the DBus API now exposes the
same data in two ways but making Describe() consistent is more important
and mirrors the OperatingSystem* class of properties.
Previously, when a library was dynamically loaded via a helper function
inside libsystemd-shared.so, the resulting dlopen ELF note was not
propagated to the invoking executable's ELF metadata.
This commit explicitly annotates each executable with the relevant
dlopen ELF notes for any optional dependencies it might potentially
load. This ensures that package managers and build systems can properly
discover runtime dependencies that are triggered indirectly.
The new polkit will return a new detail regarding a successful
authentication: the actual result type, which we can use to
see whether the user authenticated as admin. This can be used
to grant additional privileges.
This commit adds support to /etc/hostname for substitution
of $ wordlists from {/etc,/run,/usr/lib}/systemd/hostname-wordlist.
The first $ will lookup hostname-wordlist/1, the next
hostname-wordlist/2 and so on.
With that we can do a petname [1] style hostname in systemd, e.g.
below a possible expansion for a hostname template:
$-$-$-???? -> wildly-happy-octopus-92a9
The substitution of words is stable (based on machine-id) but
not persisted, it is picked on every boot via a stable file
offset so the operation is cheap. But this means that if the
wordlist changes the hostname would change. The next commit
will add the pattern to the firstboot.hostname credential which
is persistet with the resolved names to avoid this issue.
This also includes a wordlist from the "petname" project
that can be optionally installed.
Thanks to Dustin Kirkland for this wonderful project.
[1] https://github.com/dustinkirkland/petname
Teach the "tags" verb to accept arguments prefixed with '+' or '-': a
'+'-prefixed argument adds all of its (colon-separated) tags to the
configured list, a '-'-prefixed one removes them, mapping onto the new
AddAndRemoveTags() D-Bus method. The prefix applies to the whole
argument, so e.g.
hostnamectl tags +foo:bar -quux:ooo
adds foo and bar and removes quux and ooo. Plain (replace) and prefixed
(add/remove) arguments may not be mixed in one invocation.
Let's refactor things so that there's a separate xyz_is_valid() call for
each field, and make it mostly wrap string_is_safe().
This tightens the rules on some fields, mostly to refuse newlines and
some other things.
Wire up the usual SIGRTMIN+18 control signal handler and a memory
pressure event source in hostnamed's event loop, like our other
long-running daemons, so it reacts to memory pressure (e.g. by trimming
its heap) and supports the common debug/control commands. Watchdog
support via sd_event_set_watchdog() was already in place.
Add SetHostname(), SetStaticHostname(), SetPrettyHostname(), SetIconName(),
SetChassis(), SetDeployment() and SetLocation() to the io.systemd.Hostname
Varlink interface, mirroring the existing D-Bus methods of the same names.
Each takes a single nullable string "newValue" (null clears/resets the
value; the method name itself makes clear what the value is) plus the usual
allowInteractiveAuthentication polkit input, and applies the same
validation, polkit actions and /etc/hostname resp. /etc/machine-info
persistence as its D-Bus counterpart. The five machine-info setters share
a common helper. The hostname validator is reused from the D-Bus path,
translating its INVALID_ARGS error into a Varlink invalid-parameter error.
Being reachable over the socket-activated Varlink socket, these work in
early boot before the system bus is up.
Add a SetTags() method to the io.systemd.Hostname Varlink interface that
combines what the D-Bus SetTags() and AddAndRemoveTags() methods do into
a single call: it takes three tag lists, 'set', 'add' and 'remove'. If
'set' is supplied the tag list is first reset to it (an explicit empty
array clears it), otherwise the current tags are used as basis; then the
'add' tags are added and the 'remove' tags are dropped.
The set arithmetic is shared with the D-Bus AddAndRemoveTags() handler
via machine_tags_add_remove().
Being reachable over Varlink (and hence socket-activatable without the
system bus) this is usable very early at boot, before dbus-daemon is up.
Add a new D-Bus method on org.freedesktop.hostname1 that incrementally
updates the machine tags instead of replacing the whole list: it takes
two string arrays, one of tags to add and one of tags to remove, and
stores (current ∪ add) \ remove.
This avoids the read-modify-write race that clients would otherwise hit
when several of them each manage a subset of the host's tags.
Factor the tag persistence and the write-error-to-D-Bus-error mapping out
of method_set_tags() into helpers, and add a machine_tags_add_remove()
helper for the set arithmetic, all to be shared with further callers.
This is preparation for adding Varlink calls for these operations, as it
makes sense to unify log msg generation in one place (i.e. the callee)
rather than in two (the two callers, i.e. the D-Bus and the Varlink
interfaces).
For management purposes it's useful to be able to "tag" a machine with
various labels. Let's add a field for that to /etc/machine-info and make
it settable.
Fixes: #38591
The obtained strings are passed to another function, e,g, handled as a
path and opened, printed to the terminal, written to a file, saved to
udev database as udev property, exposed through DBus, passed to logger,
and so on. Hence, these should not contain any malicious characters.
This makes sure that whenever we want to show the OS name we can show
the fancy name. Thus this moves the escaping/validation of the fancy
name out of hostnamed into generic code, and then makes use of it in
sysinstall,firstboot,prompt-util.
I am really not a fan of full code lines passed to macros as parameters.
Let's get rid of the 3rd parameter of FOREACH_OPTION() hence:
1. Let's return errors just as a regular value (though a negative one),
that can be handled via a OPTION_ERROR case statement for the switch.
This normalizes handling of the error, just like any other event
returned by the option parser.
2. In order to avoid exploding the amount of boilerplate in each use
(that just propagates the error on OPTION_ERROR), let's then
introduce an explicit FOREACH_OPTION_OR_RETURN(), that returns from
the calling function on its own (and makes that clear in the name).
Together this cleans up, normalizes the logic and shortens the code.
It was requested to make the 'c', 'opt', and 'arg' params the same, i.e.
defined through the FOREACH_OPTION macro. But we can't do that easily,
because 'c' was defined in the for loop definition, and we can only
define variables of the same type in that way. Also, in some cases we
need only 'c', in other cases with need 'c' and 'arg, in some cases 'c'
and 'opt', and in other cases all three. We'd need to either
conditionalize or mark those variables with _unused_ to deal with
compiler warnings. But a different approach works quite nicely: add
state.opt and state.arg to show the current option and it's argument.
(The short names are picked on purpose to reduce verbosity since those
are used a lot.)
The updated parsing_hacks.h file uncovered a bunch of extra
things that the check-pointer-deref coccinelle script flags.
This commit fixes them to make the tree check-pointer-deref clean.
We generally want to print an error message if table_print()
fails. Add a helper function for this and use it consistently.
This does one of the three things depending on the call site:
- a no-change reformatting of the code
- change from a custom message to the generic one
- addition of the error message where previously none was printed
In the third case, the actual use impact is very small, since the
table formatting is very unlikely to fail. But if it did, we would
often return an error without any message whatsoever, which we
never want to do.
Almost all callers of table_print() specify stdout or NULL (equivalent
to stdout) as the output stream. Simplify things by not requiring the
the stream to be specified.
In almost all cases, the printing of the table is surrounded by normal
printfs() that don't do explicit flushing and for which we don't check
the output stream status. Let's simplify most callers and skip this
step. The reason is not so much to avoid the extra step itself, but
instead to avoid the _handling_ of the potential failure. We generally
only want to print an error message for ENOMEM and other "internal"
errors, so strictly speaking we should filter out the errors from the
stream. By skipping the flush&check step we implicitly do this.
After having some more experience with how this works, I think some
changes are in order. So there are a handful of preparatory patches and
then conversion of a few progs that make use of the new functionality.
The fix in 8f1751a111 made me wonder if we could automatically detect
when pointers are accessed but when this might not be safe. Systemd
is already using a lot of `assert(dst)` and this change now forces
us to use them.
So this commit (ab)uses coccinelle to flag any pointer parameter
dereference not preceded by assert(param), ASSERT_PTR(param), or an
explicit NULL check. It adds integration into meson as a new "coccinelle"
test suite (just like clang-tidy) and is run in CI. The check is not
perfect but seems a reasonable heuristic.
For this RFC commit it is scoped to a subset, it excludes 25 dirs right
now and includes around 100. About 300 warnings left. Busywork that I am
happy to do if there is agreement that it is worth it.
With this in place we would have caught the bug from 8f1751a111 in CI:
```
FAIL: check-pointer-deref.cocci found issues in systemd/src/boot:
diff -u -p systemd/src/boot/measure.c /tmp/nothing/measure.c
--- systemd/src/boot/measure.c
+++ /tmp/nothing/measure.c
@@ -312,7 +312,6 @@ EFI_STATUS tpm_log_tagged_event(
if (err != EFI_SUCCESS)
return err;
- *ret_measured = true;
return EFI_SUCCESS;
}
```
This also adds a new POINTER_MAY_BE_NULL() for the cases when the
called function will do the NULL check (like `iovec_is_set()`).
Typically, the top-level JSON object has to be an object, in any json
document we parse, hence let's add a simple way to enforce that.
Make use of this in various places.
(Note, various other JSON parsers insist on this logic right from the
beginning, but I actually thinking making this insisting optional like
this patch does it is the cleaner approach)
Currently the varlink interface for hostname is only available
after sysinit. This means it is not available until systemd-firstboot
is finished. But there is information like the boot-id in there that
is useful to get early.
My use-case is to query the system early via the varlink-http-bridge
and currently I can't get data from io.systemd.Hostname until
systemd-firstboot is completed which is a bit limiting.
So to fix it this commit sets DefaultDependencies=no on both the socket
and service units.
It also changes hostnamed.c to use
bus_open_system_watch_bind_with_description() which means we will
reconnect once dbus is available. This mimics what resolved-bus.c
is doing (and which was originally introduced in d7afd945b).
Thanks to Lennart for pointing this out.
We often have a pattern where the same verb function is used for
multiple actions. This leads to an antipattern where we figure out what
action needs to be taken from argv[0] multiple times: often once in
parse_argv() to figure out what options are allowed, then once again
implicitly in dispatch_verb(), and then again in the action verb itself.
Let's allow passing a parameter into the verb to simplify this.
This matches a pattern we have in conf-parser.h, where we have both
void *userdata (more global) and void *data (per-config item). Here,
I opted for uintptr_t userdata. It seems that most of the time we'll
want to just pass an enum value. This works OK with no casts. I also
tried a void* and union. In both cases, much more boilerplate is needed:
either a cast or a macro to help avoid compiler warnings. uintptr_t
seems generic enough to cover foreseeable usecases with no fuss.
This is a noop refactoring. See next commit for an example.
The fancy name could contain arbitrary bytes, in escaped fashion. Before
using it, let's validate that this contains only valid UTF-8.
(Note that D-Bus might kick us off the bus if we don't ensure everything
we send around is UTF-8).
(While we are at it, do the same in PID 1, even though it's not that
important there)
Addresses this issue found by @YHNdnzj:
https://github.com/systemd/systemd/pull/40367#discussion_r2714614301
Follow-up for: #40367
Zbigniew Jędrzejewski-Szmek
felix h
Nick Rosbrook
Aleksa Sarai
Yu Watanabe
Luca Boccassi
Michael Vogt
Lennart Poettering
Chitoku
seaeunlee
Mike Yuan
Lennart Poettering