core: when reloading, delay any actions on journal and dbus connections
manager_recheck_journal() and manager_recheck_dbus() would be called to early
while we were deserialiazing units, before the systemd-journald.service and
dbus.service have been deserialized. In effect we'd disable logging to the
journald and close the bus connection. The first is not very noticable, it
mostly means that logs emitted during deserialization are lost. The second is
more noticeable, because manager_recheck_dbus() would call bus_done_api() and
bus_done_system() and close dbus connections. Logging and bus connection would
then be restored later after the respective units have been deserialized.
This is easily reproduced by calling:
$ sudo gdbus call --system --dest org.freedesktop.systemd1 --object-path /org/freedesktop/systemd1 --method "org.freedesktop.systemd1.Manager.Reload"
which works fine before 8559b3b75cb, and then starts failing with:
Error: GDBus.Error:org.freedesktop.DBus.Error.NoReply: Remote peer disconnected
None of this should happen, and we should delay changing state until after
deserialization is complete when reloading. manager_reload() already included
the calls to manager_recheck_journal() and manager_recheck_dbus(), so the
connection state will be updated after deserialization during reloading is done.
Franck Bui [Thu, 15 Mar 2018 17:46:28 +0000 (18:46 +0100)]
sysusers: do not append entries after the NIS ones
The NIS-catchall entry switches from files to NIS lookup and never goes back,
so it must be the last entry in /etc/passwd (the other +/-{user,@netgroup}
entries don't have to be).
That's how the nss_compat mode for /etc/passwd (and /etc/group) traditionally
works.
It's age-old historic behaviour that the NIS entry must be the last one. It
doesn't seem to be specified somewhere, but it worked like this since very
early SunOS when NIS was first included.
udev/net-id: Fix check for address to keep interface names stable (#8458)
This was a bug inadvertently added by commit 73fc96c8ac0aa9.
The intent of the check is to "match slot address with device by
stripping the function" (as the comment above states it), for example
match network device PCI address 0000:05:00.0 (including a .0 for
function) to PCI slot address 0000:05:00, but changing that to a streq()
call prevented the match.
Change that to startswith(), which should both fix the bug and make the
intent of the check more clear and prevent unintentional bugs from being
introduced by future refactorings.
Franck Bui [Thu, 15 Mar 2018 05:23:46 +0000 (06:23 +0100)]
basic/macros: rename noreturn into _noreturn_ (#8456)
"noreturn" is reserved and can be used in other header files we include:
[ 16s] In file included from /usr/include/gcrypt.h:30:0,
[ 16s] from ../src/journal/journal-file.h:26,
[ 16s] from ../src/journal/journal-vacuum.c:31:
[ 16s] /usr/include/gpg-error.h:1544:46: error: expected ‘,’ or ‘;’ before ‘)’ token
[ 16s] void gpgrt_log_bug (const char *fmt, ...) GPGRT_ATTR_NR_PRINTF(1,2);
Here we include grcrypt.h (which in turns include gpg-error.h) *after* we
"noreturn" was defined in macro.h.
basic/calendarspec: fix assert crash when year is too large in calendarspec_from_time_t()
gmtime_r() will return NULL in that case, and we would crash.
I committed the reproducer case in fuzz-regressions/, even though we don't have
ubsan hooked up yet. Let's add it anyway in case it is useful in the future. We
actually crash anyway when compiled with asserts, so this can be easily
reproduced without ubsan.
test: run all fuzz regression tests with all sanitizers
We currently have just one sanitizer for tests, asan, but we may add more in
the future. So let's keep the loop over the sanitizers in meson.build, but
just enable all regression cases under all sanitizers. If it fails under one
of them, it might fail under a different one.
In subsequent commits I'll add test cases which might not fail under asan,
but it's good to commit them for future use.
The test names are made more verbose:
256/257 fuzz-dns-packet:oss-fuzz-5465:address OK 0.04 s
257/257 fuzz-dns-packet:issue-7888:address OK 0.03 s
Jan Janssen [Mon, 12 Mar 2018 12:33:16 +0000 (13:33 +0100)]
shutdown: Reduce log level of unmounts
There is little point in logging about unmounting errors if the
exact mountpoint will be successfully unmounted in a later retry
due unmounts below it having been removed.
Additionally, don't log those errors if we are going to switch back
to a initrd, because that one is also likely to finalize the remaining
mountpoints. If not, it will log errors then.
Yu Watanabe [Mon, 12 Mar 2018 16:18:07 +0000 (01:18 +0900)]
dhcp4: introduce new option 'duid-only' for ClientIdentifier= (#8350)
This makes users can configure DHCPv4 client with ClientIdentifier=duid-only.
If set so, then DHCP client sends only DUID as the client identifier.
This may not be RFC compliant, but some setups require this.
fuzz: allow logging to be configured, disable in fuzz-unit-file
fuzz-unit-file generated too much logs about invalid config lines. This just
slows things down and fills the logs. If necessary, it's better to rerun the
interesting cases with SYSTEMD_LOG_LEVEL=debug.
Make MANAGER_TEST_RUN_MINIMAL just allocate data structures
When running tests like test-unit-name, there is not point in setting
up the cgroup and signals and interacting with the environment. Similarly
when running fuzz testing of the parser.
Add new MANAGER_TEST_RUN_BASIC which takes the role of MANAGER_TEST_RUN_MINIMAL,
and redefine MANAGER_TEST_RUN_MINIMAL to just create the basic data structures.
Alan Jenkins [Tue, 6 Mar 2018 12:28:54 +0000 (12:28 +0000)]
login: effectively revert "open device if needed"
This replaces commit 4d3900f1b7ccce03366f9a57d259d0735c1cfbcf.
The underlying cause of issue #8291 has been fixed, so there is no reason
to paper over it any more.
But it might still be useful not to crash in the face of bad restart data.
That can cause several restarts, or maybe at some point an infinite loop
of restarts. Fail the start (or stop!) request, and write an error to the
system log. Each time reflects a user request where we fail to resume the
display server's access (or revoke it), and it can be useful if the log
shows the most recent one.
Alan Jenkins [Tue, 6 Mar 2018 15:59:38 +0000 (15:59 +0000)]
login: don't remove all devices from PID1 when only one was removed
FDSTOREREMOVE=1 removes all fds with the specified name. And we had named
the fds after the session. Better fix that.
Closes #8344.
AFAICT there's no point providing compatibility code for this transition.
No-one would be restarting logind on a system with a GUI (where the
session devices are used), because doing so has been killing the GUI, and
even causing startup of the GUI to fail leading to a restart loop.
Upgrading logind on a running system with a GUI might start being possible
after this commit (and after also fixing the display server of your
choice).
Michal Sekletar [Fri, 9 Mar 2018 22:30:32 +0000 (23:30 +0100)]
core: ignore errors from cg_create_and_attach() in test mode (#8401)
Reproducer:
$ meson build && cd build
$ ninja
$ sudo useradd test
$ sudo su test
$ ./systemd --system --test
...
Failed to create /user.slice/user-1000.slice/session-6.scope/init.scope control group: Permission denied
Failed to allocate manager object: Permission denied
Above error message is caused by the fact that user test didn't have its
own session and we tried to set up init.scope already running as user
test in the directory owned by different user.
Let's try to setup cgroup hierarchy, but if that fails return error only
when not running in the test mode.
tests: skip g_dbus_message_new_from_blob under asan
Some versions of asan report the following false positive
when strict_string_checks=1 is passed:
=================================================================
==3297==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f64e4090286 bp 0x7ffe46acd9a0 sp 0x7ffe46acd118 T0)
==3297==The signal is caused by a READ memory access.
==3297==Hint: address points to the zero page.
#0 0x7f64e4090285 in __strlen_sse2 (/lib64/libc.so.6+0xaa285)
#1 0x7f64e5a51e46 (/lib64/libasan.so.4+0x41e46)
#2 0x7f64e4e5e3a0 (/lib64/libglib-2.0.so.0+0x383a0)
#3 0x7f64e4e5e536 in g_dgettext (/lib64/libglib-2.0.so.0+0x38536)
#4 0x7f64e48fac5f (/lib64/libgio-2.0.so.0+0xc1c5f)
#5 0x7f64e4c03978 in g_type_class_ref (/lib64/libgobject-2.0.so.0+0x30978)
#6 0x7f64e4be9567 in g_object_new_with_properties (/lib64/libgobject-2.0.so.0+0x16567)
#7 0x7f64e4be9fd0 in g_object_new (/lib64/libgobject-2.0.so.0+0x16fd0)
#8 0x7f64e48fd43e in g_dbus_message_new_from_blob (/lib64/libgio-2.0.so.0+0xc443e)
#9 0x564a6aa0de52 in main ../src/libsystemd/sd-bus/test-bus-marshal.c:228
#10 0x7f64e4007009 in __libc_start_main (/lib64/libc.so.6+0x21009)
#11 0x564a6aa0a569 in _start (/home/vagrant/systemd/build/test-bus-marshal+0x5569)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xaa285) in __strlen_sse2
==3297==ABORTING
It's an external library and errors in external libraries are generally not very
useful for looking for internal bugs.
It would be better not to change the code and use standard suppression
techinques decribed at
https://clang.llvm.org/docs/AddressSanitizer.html#suppressing-reports-in-external-libraries,
but, unfortunaley, none of them seems to be able to suppress fatal errors in asan intself.
tests: skip the rest of test_mnt_id after getting any error
This mainly gets around a kernel bug making it possible to
have non-existent paths in /proc/self/mountinfo, but it should also
prevent flaky failures that can happen if something changes immediately
after or during reading /proc/self/mountinfo.
meson: avoid warning about comparison of bool and string
meson.build:2907: WARNING: Trying to compare values of different types (bool, str) using ==.
The result of this is undefined and will become a hard error in a future Meson release.
core/socket: support binary inside chroot when looking for SELinux label (#8405)
Otherwise having a .socket unit start a .service running a binary under
a chroot fails as the unit is unable to determine the SELinux label of
the binary.
Alan Jenkins [Tue, 6 Mar 2018 16:16:00 +0000 (16:16 +0000)]
login: we only allow opening character devices
We already don't allow directly opening block devices attached to the seat.
They are handled by udisks instead. Clarify the code used when restarting
logind.
Alan Jenkins [Tue, 6 Mar 2018 20:16:10 +0000 (20:16 +0000)]
login: correct comment in session_device_free()
We're not removing the pushed fd "again"; this is the only place
logind removes it from PID1. (And stopping the fd doesn't always
cause PID1 to remove the fd itself; it depends on the device type).
Suspend to Hibernate is a new sleep method that invokes suspend
for a predefined period of time before automatically waking up
and hibernating the system.
It's similar to HybridSleep however there isn't a performance
impact on every suspend cycle.
It's intended to use with systems that may have a higher power
drain in their supported suspend states to prevent battery and
data loss over an extended suspend cycle.
Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Michal Sekletar [Wed, 7 Mar 2018 15:41:41 +0000 (16:41 +0100)]
core: don't setup init.scope in test mode (#8380)
Reproducer:
$ meson build && cd build
$ ninja
$ sudo useradd test
$ sudo su test
$ ./systemd --system --test
...
Failed to create /user.slice/user-1000.slice/session-6.scope/init.scope control group: Permission denied
Failed to allocate manager object: Permission denied
Above error message is caused by the fact that user test didn't have its
own session and we tried to set up init.scope already running as user
test in the directory owned by different user.
Let's skip setting up init.scope altogether since we won't be launching
processes anyway.
core: drop unnecessary __useless_struct_to_allow_trailing_semicolon__
ISO C does not allow empty statements outside of functions, and gcc
will warn the trailing semicolons when compiling with -pedantic:
warning: ISO C does not allow extra ‘;’ outside of a function [-Wpedantic]
But our code cannot compile with -pedantic anyway, at least because
warning: ISO C does not support ‘__PRETTY_FUNCTION__’ predefined identifier [-Wpedantic]
Without -pedatnic, clang and even old gcc (3.4) generate no warnings about
those semicolons, so let's just drop __useless_struct_to_allow_trailing_semicolon__.
tests: close a leftover file descriptor in `test-fileio`
This should make it a bit easier to search for real file descriptor leaks.
```
$ valgrind --leak-check=full --track-fds=yes ./build/test-fileio
...
==29457==
==29457== FILE DESCRIPTORS: 4 open at exit.
==29457== Open file descriptor 3: /tmp/test-systemd_writing_tmpfile.lyV5Rc
==29457== at 0x4B9AD9E: open (open.c:43)
==29457== by 0x4B19B24: __gen_tempname (tempname.c:261)
==29457== by 0x4BA5CC3: mkostemp64 (mkostemp64.c:32)
==29457== by 0x48F739B: mkostemp_safe (fileio.c:1206)
==29457== by 0x10D968: test_writing_tmpfile (test-fileio.c:620)
==29457== by 0x10E930: main (test-fileio.c:767)
==29457==
```
Franck Bui [Fri, 2 Mar 2018 16:19:32 +0000 (17:19 +0100)]
tmpfiles: don't resolve pathnames when traversing recursively through directory trees
Otherwise we can be fooled if one path component is replaced underneath us.
The patch achieves that by always operating at file descriptor level (by using
*at() helpers) and by making sure we do not any path resolution when traversing
direcotry trees.
However this is not always possible, for instance when listing the content of a
directory or some operations don't provide the *at() helpers or others (such as
fchmodat()) don't have the AT_EMPTY_PATH flag. In such cases we operate on
/proc/self/fd/%i pseudo-symlink instead, which works the same for all kinds of
objects and requires no checking of type beforehand.
Also O_PATH flag is used when opening file objects in order to prevent
undesired behaviors: device nodes from reacting, automounts from
triggering, etc...