Let's better check this inside of the call than before it, so that we
never issue this while reloading, even should these calls be called due
to other reasons than just the unit notify.
This makes sure the reload state is unset a bit earlier in
manager_reload() so that we can safely call this function from there and
they do the right thing.
Follow-up for e63ebf71ed.
No need to go through the specifier_printf() if the path is already too long in
the unexpanded form (since specifiers increase the length of the string in all
practical cases).
In the oss-fuzz test case, valgrind reports:
total heap usage: 179,044 allocs, 179,044 frees, 72,687,755,703 bytes allocated
and the original config file is ~500kb. This isn't really a security issue,
since the config file has to be trusted any way, but just a matter of
preventing accidental resource exhaustion.
https://oss-fuzz.com/v2/issue/4651449704251392/6977
While at it, fix order of arguments in the neighbouring log_syntax() call.
Even if pager_open() fails, in general, we should continue the operations.
All erroneous cases in pager_open() show log message in the function.
So, it is not necessary to check the returned value.
Once upon a time shutdown.c didn't have the logic to check whether any
unmount attempts succeeded or not. So instead it kept looping for
a fixed amount and hoped all was right. Nowadays, we do know if we
changed anything during a iteration and also stop looping then, but
we still limit ourselves to FINALIZE_ATTEMPTS.
But, theoretically, we could have such a complicated and nested
setup that would survive that limit, leaving stuff around we
might actually be able to unmount. And we could also end up in a
situation where the extra loop with raised unmount error level could
be skipped too.
So let's just drop the retries logic and rely fully on the changed
flag.
It's common for sysusers files to contain quotes (in particular around the
comment/GECOS field), and using echo "..." is very likely to not work properly
in that case. Let's use <<EOF redirection. It's not bulletproof, but should
work in general.
path_is_normalized() will reject paths longer than 4095 bytes, so it's better
to not create a stack variable of unbounded size, but instead do the check first
and only then do that allocation.
Also use _cleanup_ to make things a bit shorter.
https://oss-fuzz.com/v2/issue/5424177403133952/7000
Support was killed in kernel 4.15 as well as ethtool 4.13.
Justification was lack of use by drivers and too much of a maintenance burden.
https://www.spinics.net/lists/netdev/msg443815.html
Also moved config_parse_warn_compat to conf-parser.[ch] to fix compile errors.
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 8559b3b75c, 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.
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1554578.
example.swaps with "(deleted)" does not cause bogus entries in the list now,
but a memleak in libmount instead. The memleaks is not very important since
this code is run just once.
Reported as https://github.com/karelzak/util-linux/issues/596.
$ build/test-umount
...
/* test_swap_list("/proc/swaps") */
path=/var/tmp/swap o= f=0x0 try-ro=no dev=0:0
path=/dev/dm-2 o= f=0x0 try-ro=no dev=0:0
/* test_swap_list("/home/zbyszek/src/systemd/test/test-umount/example.swaps") */
path=/some/swapfile o= f=0x0 try-ro=no dev=0:0
path=/dev/dm-2 o= f=0x0 try-ro=no dev=0:0
==26912==
==26912== HEAP SUMMARY:
==26912== in use at exit: 16 bytes in 1 blocks
==26912== total heap usage: 1,546 allocs, 1,545 frees, 149,008 bytes allocated
==26912==
==26912== 16 bytes in 1 blocks are definitely lost in loss record 1 of 1
==26912== at 0x4C31C15: realloc (vg_replace_malloc.c:785)
==26912== by 0x55C5D8C: _IO_vfscanf (in /usr/lib64/libc-2.26.so)
==26912== by 0x55D8AEC: vsscanf (in /usr/lib64/libc-2.26.so)
==26912== by 0x55D25C3: sscanf (in /usr/lib64/libc-2.26.so)
==26912== by 0x53236D0: mnt_table_parse_stream (in /usr/lib64/libmount.so.1.1.0)
==26912== by 0x53249B6: mnt_table_parse_file (in /usr/lib64/libmount.so.1.1.0)
==26912== by 0x10D157: swap_list_get (umount.c:194)
==26912== by 0x10B06E: test_swap_list (test-umount.c:34)
==26912== by 0x10B24B: main (test-umount.c:56)
==26912==
==26912== LEAK SUMMARY:
==26912== definitely lost: 16 bytes in 1 blocks
==26912== indirectly lost: 0 bytes in 0 blocks
==26912== possibly lost: 0 bytes in 0 blocks
==26912== still reachable: 0 bytes in 0 blocks
==26912== suppressed: 0 bytes in 0 blocks
This is analogous to 8d3ae2bd4c, except that now
src/core/umount.c not src/core/mount.c is converted.
Might help with https://bugzilla.redhat.com/show_bug.cgi?id=1554943, or not.
In the patch, mnt_free_tablep and mnt_free_iterp are declared twice. It'd
be nicer to define them just once in mount-setup.h, but then libmount.h would
have to be included there. libmount.h seems to be buggy, and declares some
defines which break other headers, and working around this is more pain than
the two duplicate lines. So let's live with the duplication for now.
This fixes memleak of MountPoint in mount_points_list_get() on error, not that
it matters any.
"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.
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.
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.
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.
Fixes#8072
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.
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.
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__.
