diff --git a/TODO b/TODO index 682f296d60..b8cbd3b793 100644 --- a/TODO +++ b/TODO @@ -89,6 +89,14 @@ Features: or so. (this is useful to factory reset an image, then putting it into another machine, ensuring that luks key is generated on new machine, not old) +* systemd-repart: support setting up dm-integrity with HMAC + +* systemd-repart: maybe remove half-initialized image on failure. It fails + if the output file exists, so a repeated invocation will usually fail if + something goes wrong on the way. + +* systemd-repart: drop pager mode on normal operation? + * move logind udev rules to top-level rule.d/ directory * move multiseat vid/pid matches from logind udev rule to hwdb @@ -242,8 +250,6 @@ Features: * when main nspawn supervisor process gets suspended due to SIGSTOP/SIGTTOU or so, freeze the payload too. -* repart: support setting up dm-integrity with HMAC - * add /etc/integritytab, to support dm-integrity setups. In particular those with HMAC as hash function, so that we can have a protected /home without encryption (leaving encryption to the individual dirs/homed). diff --git a/src/core/main.c b/src/core/main.c index 54ef7d182a..2f36d752bc 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1198,8 +1198,8 @@ static void bump_file_max_and_nr_open(void) { #endif #if BUMP_PROC_SYS_FS_FILE_MAX - /* The maximum the kernel allows for this since 5.2 is LONG_MAX, use that. (Previously thing where - * different but the operation would fail silently.) */ + /* The maximum the kernel allows for this since 5.2 is LONG_MAX, use that. (Previously things were + * different, but the operation would fail silently.) */ r = sysctl_writef("fs/file-max", "%li\n", LONG_MAX); if (r < 0) log_full_errno(IN_SET(r, -EROFS, -EPERM, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, "Failed to bump fs.file-max, ignoring: %m"); diff --git a/src/core/service.c b/src/core/service.c index 3f8941c1cb..ffa301980f 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -2725,7 +2725,7 @@ static int service_serialize(Unit *u, FILE *f, FDSet *fds) { return 0; } -static int service_deserialize_exec_command( +int service_deserialize_exec_command( Unit *u, const char *key, const char *value) { @@ -2783,9 +2783,6 @@ static int service_deserialize_exec_command( case STATE_EXEC_COMMAND_PATH: path = TAKE_PTR(arg); state = STATE_EXEC_COMMAND_ARGS; - - if (!path_is_absolute(path)) - return -EINVAL; break; case STATE_EXEC_COMMAND_ARGS: r = strv_extend(&argv, arg); @@ -2793,13 +2790,14 @@ static int service_deserialize_exec_command( return -ENOMEM; break; default: - assert_not_reached("Unknown error at deserialization of exec command"); - break; + assert_not_reached("Logic error in exec command deserialization"); } } if (state != STATE_EXEC_COMMAND_ARGS) return -EINVAL; + if (strv_isempty(argv)) + return -EINVAL; /* At least argv[0] must be always present. */ /* Let's check whether exec command on given offset matches data that we just deserialized */ for (command = s->exec_command[id], i = 0; command; command = command->command_next, i++) { diff --git a/src/core/service.h b/src/core/service.h index 92c1caf794..0d51fc3153 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -255,3 +255,6 @@ ServiceTimeoutFailureMode service_timeout_failure_mode_from_string(const char *s DEFINE_CAST(SERVICE, Service); #define STATUS_TEXT_MAX (16U*1024U) + +/* Only exported for unit tests */ +int service_deserialize_exec_command(Unit *u, const char *key, const char *value); diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index f1288b41a7..745366837f 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -774,7 +774,7 @@ static int run(int argc, char *argv[]) { FLAGS_SET(arg_flags, DISSECT_IMAGE_NO_PARTITION_TABLE) ? 0 : LO_FLAGS_PARTSCAN, &d); if (r < 0) - return log_error_errno(r, "Failed to set up loopback device: %m"); + return log_error_errno(r, "Failed to set up loopback device for %s: %m", arg_image); r = dissect_image_and_warn( d->fd, diff --git a/src/portable/portable.c b/src/portable/portable.c index 0799bff53d..53c4d8e25b 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -376,7 +376,7 @@ static int portable_extract_by_path( return r; } else if (r < 0) - return log_debug_errno(r, "Failed to set up loopback device: %m"); + return log_debug_errno(r, "Failed to set up loopback device for %s: %m", path); else { _cleanup_(dissected_image_unrefp) DissectedImage *m = NULL; _cleanup_(rmdir_and_freep) char *tmpdir = NULL; diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index b425014457..05301ef16d 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -2741,7 +2741,7 @@ int mount_image_privately_interactively( FLAGS_SET(flags, DISSECT_IMAGE_NO_PARTITION_TABLE) ? 0 : LO_FLAGS_PARTSCAN, &d); if (r < 0) - return log_error_errno(r, "Failed to set up loopback device: %m"); + return log_error_errno(r, "Failed to set up loopback device for %s: %m", image); r = dissect_image_and_warn(d->fd, image, &verity, NULL, d->uevent_seqnum_not_before, d->timestamp_not_before, flags, &dissected_image); if (r < 0) diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index c5fdf99aa7..76a21afea6 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -525,7 +525,7 @@ static int merge_subprocess(Hashmap *images, const char *workspace) { r = loop_device_make_by_path(img->path, O_RDONLY, 0, &d); if (r < 0) - return log_error_errno(r, "Failed to set up loopback device: %m"); + return log_error_errno(r, "Failed to set up loopback device for %s: %m", img->path); r = dissect_image_and_warn( d->fd, diff --git a/src/test/meson.build b/src/test/meson.build index e077c8e03f..991c0f027d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -133,6 +133,17 @@ tests += [ [['src/test/test-serialize.c']], + [['src/test/test-unit-serialize.c'], + [libcore, + libshared], + [threads, + librt, + libseccomp, + libselinux, + libmount, + libblkid], + core_includes], + [['src/test/test-utf8.c']], [['src/test/test-dev-setup.c']], diff --git a/src/test/test-unit-serialize.c b/src/test/test-unit-serialize.c new file mode 100644 index 0000000000..58a0d9dfd4 --- /dev/null +++ b/src/test/test-unit-serialize.c @@ -0,0 +1,68 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "rm-rf.h" +#include "service.h" +#include "tests.h" + +#define EXEC_START_ABSOLUTE \ + "ExecStart 0 /bin/sh \"sh\" \"-e\" \"-x\" \"-c\" \"systemctl --state=failed --no-legend --no-pager >/failed ; systemctl daemon-reload ; echo OK >/testok\"" +#define EXEC_START_RELATIVE \ + "ExecStart 0 sh \"sh\" \"-e\" \"-x\" \"-c\" \"systemctl --state=failed --no-legend --no-pager >/failed ; systemctl daemon-reload ; echo OK >/testok\"" + +static void test_deserialize_exec_command_one(Manager *m, const char *key, const char *line, int expected) { + _cleanup_(unit_freep) Unit *u = NULL; + int r; + + assert_se(unit_new_for_name(m, sizeof(Service), "test.service", &u) >= 0); + + r = service_deserialize_exec_command(u, key, line); + log_debug("[%s] → %d (expected: %d)", line, r, expected); + assert(r == expected); + + /* Note that the command doesn't match any command in the empty list of commands in 's', so it is + * always rejected with "Current command vanished from the unit file", and we don't leak anything. */ +} + +static void test_deserialize_exec_command(void) { + _cleanup_(manager_freep) Manager *m = NULL; + int r; + + log_info("/* %s */", __func__); + + r = manager_new(UNIT_FILE_USER, MANAGER_TEST_RUN_MINIMAL, &m); + if (manager_errno_skip_test(r)) { + log_notice_errno(r, "Skipping test: manager_new: %m"); + return; + } + + assert_se(r >= 0); + + test_deserialize_exec_command_one(m, "main-command", EXEC_START_ABSOLUTE, 0); + test_deserialize_exec_command_one(m, "main-command", EXEC_START_RELATIVE, 0); + test_deserialize_exec_command_one(m, "control-command", EXEC_START_ABSOLUTE, 0); + test_deserialize_exec_command_one(m, "control-command", EXEC_START_RELATIVE, 0); + + test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /bin/sh \"sh\"", 0); + test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /no/command ", -EINVAL); + test_deserialize_exec_command_one(m, "control-command", "ExecStart 0 /bad/quote \"", -EINVAL); + test_deserialize_exec_command_one(m, "control-command", "ExecStart s /bad/id x y z", -EINVAL); + test_deserialize_exec_command_one(m, "control-command", "ExecStart 11", -EINVAL); + test_deserialize_exec_command_one(m, "control-command", "ExecWhat 11 /a/b c d e", -EINVAL); +} + +int main(int argc, char *argv[]) { + _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL; + int r; + + test_setup_logging(LOG_DEBUG); + + r = enter_cgroup_subroot(NULL); + if (r == -ENOMEDIUM) + return log_tests_skipped("cgroupfs not available"); + + assert_se(runtime_dir = setup_fake_runtime_dir()); + + test_deserialize_exec_command(); + + return EXIT_SUCCESS; +} diff --git a/test/TEST-02-UNITTESTS/test.sh b/test/TEST-02-UNITTESTS/test.sh index 2bfe41a42b..f7545b7620 100755 --- a/test/TEST-02-UNITTESTS/test.sh +++ b/test/TEST-02-UNITTESTS/test.sh @@ -39,7 +39,7 @@ check_result_nspawn() { save_journal "$workspace/var/log/journal" _umount_dir "${initdir:?}" - [[ -n "${TIMED_OUT:=}" ]] && ret=$((ret + 1)) + [[ -n "${TIMED_OUT:=}" ]] && ret=1 return $ret } @@ -67,7 +67,7 @@ check_result_qemu() { save_journal "$initdir/var/log/journal" _umount_dir "$initdir" - [[ -n "${TIMED_OUT:=}" ]] && ret=$((ret + 1)) + [[ -n "${TIMED_OUT:=}" ]] && ret=1 return $ret } diff --git a/test/TEST-24-CRYPTSETUP/test.sh b/test/TEST-24-CRYPTSETUP/test.sh index e4d99d10b9..109d403568 100755 --- a/test/TEST-24-CRYPTSETUP/test.sh +++ b/test/TEST-24-CRYPTSETUP/test.sh @@ -26,7 +26,7 @@ check_result_qemu() { [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" echo "${JOURNAL_LIST:-No journals were saved}" - test -s "$TESTDIR/failed" && ret=$((ret + 1)) + test -s "$TESTDIR/failed" && ret=1 return $ret } diff --git a/test/TEST-55-OOMD/test.sh b/test/TEST-55-OOMD/test.sh index 9f7a11aea4..6f7e776c3c 100755 --- a/test/TEST-55-OOMD/test.sh +++ b/test/TEST-55-OOMD/test.sh @@ -19,52 +19,4 @@ EOF ) } -check_result_nspawn() { - local workspace="${1:?}" - local ret=1 - local journald_report="" - local pids="" - - [[ -e "$workspace/testok" ]] && ret=0 - if [[ -e "$workspace/skipped" ]]; then - echo "TEST-56-OOMD was skipped:" - cat "$workspace/skipped" - ret=0 - fi - - [[ -f "$workspace/failed" ]] && cp -a "$workspace/failed" "${TESTDIR:?}" - save_journal "$workspace/var/log/journal" - [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" - echo "${JOURNAL_LIST:-No journals were saved}" - - test -s "$TESTDIR/failed" && ret=$((ret + 1)) - [ -n "${TIMED_OUT:=}" ] && ret=$((ret + 1)) - check_asan_reports "$workspace" || ret=$((ret + 1)) - _umount_dir "${initdir:?}" - return $ret -} - -check_result_qemu() { - local ret=1 - - mount_initdir - [[ -e "${initdir:?}/testok" ]] && ret=0 - if [[ -e "$initdir/skipped" ]]; then - echo "TEST-56-OOMD was skipped:" - cat "$initdir/skipped" - ret=0 - fi - - [[ -f "$initdir/failed" ]] && cp -a "$initdir/failed" "${TESTDIR:?}" - save_journal "$initdir/var/log/journal" - check_asan_reports "$initdir" || ret=$((ret + 1)) - _umount_dir "$initdir" - [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" - echo "${JOURNAL_LIST:-No journals were saved}" - - test -s "$TESTDIR/failed" && ret=$((ret + 1)) - [ -n "${TIMED_OUT:=}" ] && ret=$((ret + 1)) - return $ret -} - do_test "$@" 55 diff --git a/test/TEST-58-REPART/test.sh b/test/TEST-58-REPART/test.sh index d94a9cbfdc..bb08c4fc20 100755 --- a/test/TEST-58-REPART/test.sh +++ b/test/TEST-58-REPART/test.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash set -e TEST_DESCRIPTION="test systemd-repart" +TEST_NO_NSPAWN=1 . $TEST_BASE_DIR/test-functions -do_test "$@" 56 +do_test "$@" 58 diff --git a/test/test-functions b/test/test-functions index e8d2a5876b..f6d1c9e0bb 100644 --- a/test/test-functions +++ b/test/test-functions @@ -632,7 +632,8 @@ setup_basic_environment() { install_keymaps install_terminfo install_execs - install_fsck + install_fs_tools + install_modules install_plymouth install_debug_tools install_ld_so_conf @@ -833,13 +834,28 @@ EOF chmod 0755 "$strace_wrapper" } -install_fsck() { +install_fs_tools() { dinfo "Install fsck" dracut_install /sbin/fsck* dracut_install -o /bin/fsck* # fskc.reiserfs calls reiserfsck. so, install it dracut_install -o reiserfsck + + # we use mkfs in system-repart tests + dracut_install /sbin/mkfs.ext4 + dracut_install /sbin/mkfs.vfat +} + +install_modules() { + dinfo "Install modules" + + instmods loop + instmods vfat + + if [[ "$LOOKS_LIKE_SUSE" ]]; then + instmods ext4 + fi } install_dmevent() { @@ -1102,42 +1118,66 @@ save_journal() { JOURNAL_LIST="$(ls -l "$dest"*)" } -check_result_nspawn() { +check_result_common() { local workspace="${1:?}" - local ret=1 - local journald_report="" - local pids="" - [[ -e "$workspace/testok" ]] && ret=0 - [[ -f "$workspace/failed" ]] && cp -a "$workspace/failed" "${TESTDIR:?}" + local ret + + if [ -s "$workspace/failed" ]; then + # …/failed only counts if non-empty + ls -l "$workspace/failed" + cp -a "$workspace/failed" "${TESTDIR:?}/" + ret=1 + elif [ -e "$workspace/testok" ]; then + # …/testok always counts (but with lower priority than …/failed) + ret=0 + elif [ -e "$workspace/skipped" ]; then + # …/skipped always counts (a message is expected) + echo "${TESTNAME:?} was skipped:" + cat "$workspace/skipped" + ret=0 + elif [ -n "$TIMED_OUT" ]; then + echo "${TESTNAME:?} timed out!" + ret=2 + else + echo "${TESTNAME:?} did not report a result!" + fi + save_journal "$workspace/var/log/journal" - [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" - echo "${JOURNAL_LIST:-"No journals were saved"}" - test -s "$TESTDIR/failed" && ret=$((ret+1)) - [ -n "$TIMED_OUT" ] && ret=$((ret+1)) - check_asan_reports "$workspace" || ret=$((ret+1)) + + check_asan_reports "$workspace" || ret=3 + if [ -d "${ARTIFACT_DIRECTORY}" ] && [ -f "$workspace/strace.out" ]; then cp "$workspace/strace.out" "${ARTIFACT_DIRECTORY}/" fi + + [ -f "$TESTDIR/failed" ] && cat "$TESTDIR/failed" + echo "${JOURNAL_LIST:-"No journals were saved"}" + + return $ret +} + +check_result_nspawn() { + local workspace="${1:?}" + local ret + + check_result_common "${workspace}" + ret=$? + _umount_dir "${initdir:?}" + return $ret } # can be overridden in specific test check_result_qemu() { - local ret=1 + local ret mount_initdir - [[ -e "${initdir:?}/testok" ]] && ret=0 - [[ -f "$initdir/failed" ]] && cp -a "$initdir/failed" "${TESTDIR:?}" - save_journal "$initdir/var/log/journal" - check_asan_reports "$initdir" || ret=$((ret+1)) - if [ -d "${ARTIFACT_DIRECTORY}" ] && [ -f "$initdir/strace.out" ]; then - cp "$initdir/strace.out" "${ARTIFACT_DIRECTORY}/" - fi - _umount_dir "$initdir" - [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" - echo "${JOURNAL_LIST:-"No journals were saved"}" - test -s "$TESTDIR/failed" && ret=$((ret+1)) - [ -n "$TIMED_OUT" ] && ret=$((ret+1)) + + check_result_common "${initdir:?}" + ret=$? + + _umount_dir "${initdir:?}" + return $ret } @@ -2192,7 +2232,7 @@ instmods() { ((ret+=$?)) ;; esac - return $ret + return "$ret" } local mod mpargs @@ -2220,7 +2260,6 @@ setup_suse() { ln -fs ../usr/bin/systemctl "${initdir:?}/bin/" ln -fs ../usr/lib/systemd "$initdir/lib/" inst_simple "/usr/lib/systemd/system/haveged.service" - instmods ext4 } _umount_dir() { diff --git a/test/units/testsuite-58.service b/test/units/testsuite-58.service index d8ad589ca0..b962eb0f0f 100644 --- a/test/units/testsuite-58.service +++ b/test/units/testsuite-58.service @@ -1,5 +1,5 @@ [Unit] -Description=TEST-56-EXIT-TYPE +Description=TEST-58-REPART [Service] ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh diff --git a/test/units/testsuite-58.sh b/test/units/testsuite-58.sh index 772f1b78d4..13f02f01e3 100755 --- a/test/units/testsuite-58.sh +++ b/test/units/testsuite-58.sh @@ -1,21 +1,28 @@ #!/usr/bin/env bash set -eux +set -o pipefail + +if ! command -v systemd-repart &>/dev/null; then + echo "no systemd-repart" >/skipped + exit 0 +fi export SYSTEMD_LOG_LEVEL=debug export PAGER=cat +rm -f /var/tmp/testsuite-58.img /var/tmp/testsuite-58.2.img /tmp/testsuite-58.dump mkdir -p /tmp/testsuite-58-defs/ # First part: create a disk image and verify its in order -cat > /tmp/testsuite-58-defs/esp.conf </tmp/testsuite-58-defs/esp.conf < /tmp/testsuite-58-defs/usr.conf </tmp/testsuite-58-defs/usr.conf < /tmp/testsuite-58-defs/root.conf </tmp/testsuite-58-defs/root.conf < /tmp/testsuite-58.dump +sfdisk --dump /var/tmp/testsuite-58.img >/tmp/testsuite-58.dump grep -qxF '/var/tmp/testsuite-58.img1 : start= 2048, size= 20480, type=C12A7328-F81F-11D2-BA4B-00A0C93EC93B, uuid=39107B09-615D-48FB-BA37-C663885FCE67, name="esp"' /tmp/testsuite-58.dump grep -qxF '/var/tmp/testsuite-58.img2 : start= 22528, size= 20480, type=4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709, uuid=60F33797-1D71-4DCB-AA6F-20564F036CD0, name="root-x86-64"' /tmp/testsuite-58.dump @@ -41,31 +52,36 @@ grep -qxF '/var/tmp/testsuite-58.img3 : start= 43008, size= 20480, t # Second part, duplicate it with CopyBlocks=auto -cat > /tmp/testsuite-58-defs/esp.conf </tmp/testsuite-58-defs/esp.conf < /tmp/testsuite-58-defs/usr.conf </tmp/testsuite-58-defs/usr.conf < /tmp/testsuite-58-defs/root.conf </tmp/testsuite-58-defs/root.conf </testok