Eric Anderson [Sat, 2 May 2020 22:54:24 +0000 (15:54 -0700)]
socket-proxy: Support exit-on-idle
This adds the --exit-idle-time argument that causes
systemd-socket-proxyd to exit when there has been an idle period. An
open connection prevents the idle period from starting, even if there is
no activity on that connection.
When combined with another service that uses StopWhenUnneeded=, the
proxy exiting can trigger a resource-intensive process to exit. So
although the proxy may consume minimal resources, significant resources
can be saved indirectly.
The old version of this command will delete everything under the new home directory (including `.identity`), rendering the directory unusable with homed.
Having taken a look at https://github.com/systemd/systemd/runs/645252074?check_suite_focus=true
where fuzz-journal-remote failed with
```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==16==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f864f98948e bp 0x7ffde5c6b7c0 sp 0x7ffde5c6b560 T0)
==16==The signal is caused by a READ memory access.
==16==Hint: address points to the zero page.
SCARINESS: 10 (null-deref)
#0 0x7f864f98948e in output_short /work/build/../../src/systemd/src/shared/logs-show.c
#1 0x7f864f984624 in show_journal_entry /work/build/../../src/systemd/src/shared/logs-show.c:1154:15
#2 0x7f864f984b63 in show_journal /work/build/../../src/systemd/src/shared/logs-show.c:1239:21
#3 0x4cabab in LLVMFuzzerTestOneInput /work/build/../../src/systemd/src/fuzz/fuzz-journal-remote.c:67:21
#4 0x51fd16 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:556:15
#5 0x51c330 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/libfuzzer/FuzzerLoop.cpp:470:3
#6 0x523700 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/libfuzzer/FuzzerLoop.cpp:765:7
#7 0x5246cd in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/libfuzzer/FuzzerLoop.cpp:792:3
#8 0x4de3d1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:824:6
#9 0x4cfb47 in main /src/libfuzzer/FuzzerMain.cpp:19:10
#10 0x7f864e69782f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#11 0x41f2a8 in _start (out/fuzz-journal-remote+0x41f2a8)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /work/build/../../src/systemd/src/shared/logs-show.c in output_short
==16==ABORTING
MS: 0 ; base unit: 0000000000000000000000000000000000000000
0x44,0x3d,0xa,0x5f,0x5f,0x52,0x45,0x41,0x4c,0x54,0x49,0x4d,0x45,0x5f,0x54,0x49,0x4d,0x45,0x53,0x54,0x41,0x4d,0x50,0x3d,0x31,0xa,0xa,
D=\x0a__REALTIME_TIMESTAMP=1\x0a\x0a
artifact_prefix='./'; Test unit written to ./crash-d635b9dd31cceff3c912fd45e1a58d7e90f0ad73
Base64: RD0KX19SRUFMVElNRV9USU1FU1RBTVA9MQoK
```
I was wondering why it hadn't been caught by the compiler even though clang should have failed to compile it with
```
../src/shared/logs-show.c:624:25: warning: null passed to a callee that requires a non-null argument [-Wnonnull]
print_multiline(f, 4 + fieldlen + 1, 0, OUTPUT_FULL_WIDTH, 0, false,
^
../src/shared/logs-show.c:161:24: note: callee declares array parameter as static here
size_t highlight[static 2]) {
^ ~~~~~~~~~~
../src/shared/logs-show.c:1239:21: warning: null passed to a callee that requires a non-null argument [-Wnonnull]
r = show_journal_entry(f, j, mode, n_columns, flags, NULL, NULL, ellipsized);
^ ~~~~
../src/shared/logs-show.c:1133:30: note: callee declares array parameter as static here
const size_t highlight[static 2],
^ ~~~~~~~~~~
2 warnings generated.
```
Given that judging by https://github.com/systemd/systemd/issues/13039 it doesn't seem to be
the first time issues like that have been missed I think it would be better to turn nonnull on
and get around false positives on a case-by-case basis with DISABLE_WARNING_NONNULL .. REENABLE_WARNING
Let's use "!*" instead of "!!" as invalid password string.
Generally, any invalid password string can be used to for locking an
account, according to shadow(5). To temporarily lock a password of an
account it is commonly implemented to prefix the original password with
a single "!", so that it can later on be unlocked again by removing the
"!", restoring the original password. Thus, the "!" marker is an
indicator for a locked password; the act of prefixing "!" to a
password string is the locking operation; and the removal of a "!"
prefix is the unlock operation. (This is also suggested in shadow(5)).
If we want to entirely lock an account we previously used "!!" as
password string. This is nice since it indicates the password is locked.
However, it is less than ideal, since applying the password unlock
operation once will change the string to "!", which is still a locked
password. Unlocking the password a second time will result in "", i.e.
the empty password, which will in many cases allow logging in without
password. And that's a problem. Hopefully, tools do not allow such
duplicate unlocking, but it's still not a nice property.
By changing our password string to "!*" we get different behaviour: the
password will appear locked. When it is unlocked the password is "*"
which is an invalid password. In that case the password is hence
unlocked but invalid, which is a much better state to be in than the
above.
This is paranoia hardening. Not more. There's no report that anyone
every unlocked an account twice and people could log in.
man: add description of org.freedesktop.LogControl1
I'm not sure if the LogTarget property is sufficiently general to be made into
a property that can be generally implemented. It is very closely tied to the internal
systemd logic. The other two seem fine thoough.
man: use --bus-introspect to autogenerate dbus api docs
This has the advantage that the executables are always in place and we don't
need any units to exist on the bus, so we can eventually hook this up into
a normal build system. (Probably as a build time check.)
In all the other cases, I think the code was clearer with the static table.
Here, not so much. And because of the existing dump code, the vtables cannot
be made static and need to remain exported. I still think it's worth to do the
change to have the cmdline introspection, but I'm disappointed with how this
came out.
units: change description of systemd-logind.service
"Login Service" doesn''t explain much, esp. considering that logind is actually is
for logins. I think "User Login Management" is better, but not that great either.
Suggestions welcome.
sd-bus: add helper struct for interface definitions
The idea is to have a static table that defines the dbus API. The vtable is
defined right next to the interface name and path because they are logically
connected.
systemctl: do not show preset state for "static"/"alias"/"generated"/"transient" units
This modifies list-unit-files and status.
The output of list-unit-files would contain various lines with "static
disabled", which is just misleading, because systemctl preset will not touch
those files, so the preset configuration is not relevant. Similarly, for
"generated" and "transient" units, preset state is irrelevant. For "alias"
lines, the preset for the alias target would be used. In all cases let's just
skip preset status in those cases.
(It turns out that most of the services on this list that are enabled without a
corresponding preset are against rpm packaging and do direct 'systemctl enable'
calls in %post, instead of going through presets as they are supposed to.)
For units which are aliases of other units, reporting preset status as
"enabled" is rather misleading. For example, dbus.service is an alias of
dbus-broker.service. In list-unit-files we'd show both as "enabled". In
particular, systemctl preset ignores aliases, so showing any preset status at
all is always going to be misleading. Let's introduce a new state "alias" and
use that for all aliases.
I was trying to avoid adding a new state, to keep compatibility with previous
behaviour, but for alias unit files it simply doesn't seem very useful to show
any of the existing states. It seems that the clearly showing that those are
aliases for other units will be easiest to understand for users.
When doing list-unit-files with --root, we would re-read the preset
list for every unit. This uses a cache to only do it once. The time
for list-unit-files goes down by about ~30%.
unit_file_query_preset() is also called from src/core/. This patch does not
touch that path, since the saving there are smaller, since preset status is
only read on demand over dbus, and caching would be more complicated.
5238d9a83a52 renames this to exit-status, but systemd.service was not
updated.
The rest of the doc seems a bit inconsistent in its use of the terms
"exit code" and "exit status", but it's not that confusing, so leave
those alone for now.
It is super confusing when a command does not support --root, and is called
with it specified, and returns some bogus results. Let's just catch this
early and refuse.
ianhi [Tue, 5 May 2020 00:27:16 +0000 (20:27 -0400)]
correct evdev dimensions for T490 trackpad
The original source of these dimensions had swapped the x y dimension
arguments to touchpad-edge-detector. The correct dimensions are
x = 100 mm, y = 68 mm
Assuming one wants `/` to be read-only, DNS and `/etc/localtime` to
work. One way to do it is to create an overlay filesystem in `/etc/`.
However, systemd-nspawn tries to create `/etc/resolv.conf` and
`/etc/localtime` before mounting the custom paths, while `/` (and, by
extension, `/etc`) is read-only. Thus it fails to create those files.
Mounting custom paths before modifying anything in `/etc/` makes this
possible.
Full example:
```
$ debootstrap buster /var/lib/machines/t1 http://deb.debian.org/debian
$ systemd-nspawn --private-users=false --timezone=copy --resolv-conf=copy-host --read-only --tmpfs=/var --tmpfs=/run --overlay="+/etc::/etc" -D /var/lib/machines/t1 ping -c 1 example.com
Spawning container t1 on /var/lib/machines/t1.
Press ^] three times within 1s to kill container.
ping: example.com: Temporary failure in name resolution
Container t1 failed with error code 130.
```
With the patch:
```
$ sudo ./build/systemd-nspawn --private-users=false --timezone=copy --resolv-conf=copy-host --read-only --tmpfs=/var --tmpfs=/run --overlay="+/etc::/etc" -D /var/lib/machines/t1 ping -qc 1 example.com
Spawning container t1 on /var/lib/machines/t1.
Press ^] three times within 1s to kill container.
PING example.com (93.184.216.34) 56(84) bytes of data.
calendarspec: be more graceful with two kinds of calendar expressions
This changes the calendarspec parser to allow expressions such as
"00:05..05", i.e. a range where start and end is the same. It also
allows expressions such as "00:1-2/3", i.e. where the repetition value
does not fit even once in the specified range. With this patch both
cases will now be optimized away, i.e. the range is removed and a fixed
value is used, which is functionally equivalent.
See #15030 for an issue where the inability to parse such expressions
caused confusion.
I think it's probably better to accept these gracefully and optimizing
them away instead of refusing them with a plain EINVAL. With a tool such
as "systemd-analyze" calendar it should be easy to figure out the
normalized form with the redundant bits optimized away.
Let's try to mangle table contents a bit to make them more suitable as
JSON field names. Specifically when we see "foo bar" convert this to
"foo_bar" as field name, as variable/field names are generally assumed
to be without spaces.
Michal Koutný [Fri, 1 May 2020 16:38:10 +0000 (18:38 +0200)]
test: Fix build with !HAVE_LZ4 && HAVE_XZ
HUGE_SIZE was defined inconsistently.
> In file included from ../src/basic/alloc-util.h:9,
> from ../src/journal/test-compress.c:9:
> ../src/journal/test-compress.c: In function ‘main’:
> ../src/journal/test-compress.c:280:33: error: ‘HUGE_SIZE’ undeclared (first use in this function)
> 280 | assert_se(huge = malloc(HUGE_SIZE));
man: sd_notify() race is gone with sd_notify_barrier()
Add note for change of behaviour in systemd-notify, where parent pid trick
is only used when --no-block is passed, and with enough privileges ofcourse.
This adds the sd_notify_barrier function, to allow users to synchronize against
the reception of sd_notify(3) status messages. It acts as a synchronization
point, and a successful return gurantees that all previous messages have been
consumed by the manager. This can be used to eliminate race conditions where
the sending process exits too early for systemd to associate its PID to a
cgroup and attribute the status message to a unit correctly.
systemd-notify now uses this function for proper notification delivery and be
useful for NotifyAccess=all units again in user mode, or in cases where it
doesn't have a control process as parent.
Dan Streetman [Sun, 26 Apr 2020 15:19:55 +0000 (11:19 -0400)]
test: find path for systemd-journal-remote
As Debian/Ubuntu use /lib/systemd instead of /usr/lib/systemd,
add systemd-journal-remote to the list of programs that test-functions
detects the correct path to, and replace its direct usage with
$SYSTEMD_JOURNAL_REMOTE
Also use $JOURNALCTL instead of journalctl.
Also minor correction in install_plymouth() to look in /lib/... as
well as /usr/lib/... and /etc/...
A service can specify FDSTORE=1 FDPOLL=0 to request that PID1 does not
poll the fd to remove them on error. If set, fds will only be removed on
FDSTOREREMOVE=1 or when the service is done.
Michal Sekletár [Wed, 29 Apr 2020 15:53:43 +0000 (17:53 +0200)]
core: introduce support for cgroup freezer
With cgroup v2 the cgroup freezer is implemented as a cgroup
attribute called cgroup.freeze. cgroup can be frozen by writing "1"
to the file and kernel will send us a notification through
"cgroup.events" after the operation is finished and processes in the
cgroup entered quiescent state, i.e. they are not scheduled to
run. Writing "0" to the attribute file does the inverse and process
execution is resumed.
This commit exposes above low-level functionality through systemd's DBus
API. Each unit type must provide specialized implementation for these
methods, otherwise, we return an error. So far only service, scope, and
slice unit types provide the support. It is possible to check if a
given unit has the support using CanFreeze() DBus property.
Note that DBus API has a synchronous behavior and we dispatch the reply
to freeze/thaw requests only after the kernel has notified us that
requested operation was completed.