ukify: drop NX bit from UKI if kernel doesn't have it

If the kernel is not NX_COMPAT ready (W^X memory compatible) then the
UKI should not be marked as NX_COMPAT ready either, as the kernel
section is the loadable code in the image.

https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/
https://www.kraxel.org/blog/2023/12/uefi-nx-linux-boot/

While the sd-stub EFI code itself is NX ready, it is more useful
to think of it as one unit of execution together with the kernel
it embeds, as that's what it is used for.

Fixes https://github.com/systemd/systemd/issues/38545

profile.d/80-systemd-osc-context.sh

@@ -53,7 +53,7 @@ __systemd_osc_context_precmdline() {
     read -r systemd_osc_context_cmd_id </proc/sys/kernel/random/uuid
 }
 
-if [[ -n "${BASH_VERSION:-}" ]]; then
+if [[ -n "${BASH_VERSION:-}" ]] && [[ "${TERM:-}" != "dumb" ]]; then
     # Whenever a new prompt is shown close the previous command, and prepare new command
     PROMPT_COMMAND+=(__systemd_osc_context_precmdline)
 

src/core/cgroup.c

@@ -2088,9 +2088,6 @@ static int unit_update_cgroup(
         if (!UNIT_HAS_CGROUP_CONTEXT(u))
                 return 0;
 
-        if (u->freezer_state != FREEZER_RUNNING)
-                return log_unit_error_errno(u, SYNTHETIC_ERRNO(EBUSY), "Cannot realize cgroup for frozen unit.");
-
         r = unit_get_cgroup_path_with_fallback(u, &cgroup);
         if (r < 0)
                 return log_unit_error_errno(u, r, "Failed to get cgroup path: %m");

src/core/exec-invoke.c

@@ -128,26 +128,6 @@ static int flag_fds(
         return 0;
 }
 
-static bool is_terminal_input(ExecInput i) {
-        return IN_SET(i,
-                      EXEC_INPUT_TTY,
-                      EXEC_INPUT_TTY_FORCE,
-                      EXEC_INPUT_TTY_FAIL);
-}
-
-static bool is_terminal_output(ExecOutput o) {
-        return IN_SET(o,
-                      EXEC_OUTPUT_TTY,
-                      EXEC_OUTPUT_KMSG_AND_CONSOLE,
-                      EXEC_OUTPUT_JOURNAL_AND_CONSOLE);
-}
-
-static bool is_kmsg_output(ExecOutput o) {
-        return IN_SET(o,
-                      EXEC_OUTPUT_KMSG,
-                      EXEC_OUTPUT_KMSG_AND_CONSOLE);
-}
-
 static int open_null_as(int flags, int nfd) {
         int fd;
 
@@ -252,8 +232,8 @@ static int connect_logger_as(
                 context->syslog_priority,
                 !!context->syslog_level_prefix,
                 false,
-                is_kmsg_output(output),
-                is_terminal_output(output)) < 0)
+                exec_output_is_kmsg(output),
+                exec_output_is_terminal(output)) < 0)
                 return -errno;
 
         return move_fd(TAKE_FD(fd), nfd, false);
@@ -325,7 +305,7 @@ static int fixup_input(
 
         std_input = context->std_input;
 
-        if (is_terminal_input(std_input) && !apply_tty_stdin)
+        if (exec_input_is_terminal(std_input) && !apply_tty_stdin)
                 return EXEC_INPUT_NULL;
 
         if (std_input == EXEC_INPUT_SOCKET && socket_fd < 0)
@@ -531,7 +511,7 @@ static int setup_output(
                 if (e == EXEC_OUTPUT_INHERIT &&
                     o == EXEC_OUTPUT_INHERIT &&
                     i == EXEC_INPUT_NULL &&
-                    !is_terminal_input(context->std_input) &&
+                    !exec_input_is_terminal(context->std_input) &&
                     getppid() != 1)
                         return fileno;
 
@@ -543,7 +523,7 @@ static int setup_output(
 
         } else if (o == EXEC_OUTPUT_INHERIT) {
                 /* If input got downgraded, inherit the original value */
-                if (i == EXEC_INPUT_NULL && is_terminal_input(context->std_input))
+                if (i == EXEC_INPUT_NULL && exec_input_is_terminal(context->std_input))
                         return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno);
 
                 /* If the input is connected to anything that's not a /dev/null or a data fd, inherit that... */
@@ -564,7 +544,7 @@ static int setup_output(
                 return open_null_as(O_WRONLY, fileno);
 
         case EXEC_OUTPUT_TTY:
-                if (is_terminal_input(i))
+                if (exec_input_is_terminal(i))
                         return RET_NERRNO(dup2(STDIN_FILENO, fileno));
 
                 return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno);
@@ -4880,8 +4860,8 @@ static void prepare_terminal(
         assert(p);
 
         /* We only try to reset things if we there's the chance our stdout points to a TTY */
-        if (!(is_terminal_output(context->std_output) ||
-              (context->std_output == EXEC_OUTPUT_INHERIT && is_terminal_input(context->std_input)) ||
+        if (!(context->std_output == EXEC_OUTPUT_TTY ||
+              (context->std_output == EXEC_OUTPUT_INHERIT && exec_input_is_terminal(context->std_input)) ||
               context->std_output == EXEC_OUTPUT_NAMED_FD ||
               p->stdout_fd >= 0))
                 return;
@@ -4921,10 +4901,7 @@ static int setup_term_environment(const ExecContext *context, char ***env) {
                 return 0;
 
         /* Do we need $TERM at all? */
-        if (!is_terminal_input(context->std_input) &&
-            !is_terminal_output(context->std_output) &&
-            !is_terminal_output(context->std_error) &&
-            !context->tty_path)
+        if (!exec_context_has_tty(context))
                 return 0;
 
         const char *tty_path = exec_context_tty_path(context);

src/core/execute.c

@@ -65,20 +65,6 @@
 #include "utmp-wtmp.h"
 #include "vpick.h"
 
-static bool is_terminal_input(ExecInput i) {
-        return IN_SET(i,
-                      EXEC_INPUT_TTY,
-                      EXEC_INPUT_TTY_FORCE,
-                      EXEC_INPUT_TTY_FAIL);
-}
-
-static bool is_terminal_output(ExecOutput o) {
-        return IN_SET(o,
-                      EXEC_OUTPUT_TTY,
-                      EXEC_OUTPUT_KMSG_AND_CONSOLE,
-                      EXEC_OUTPUT_JOURNAL_AND_CONSOLE);
-}
-
 const char* exec_context_tty_path(const ExecContext *context) {
         assert(context);
 
@@ -151,8 +137,7 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *pa
 
         if (parameters && parameters->stdout_fd >= 0 && isatty_safe(parameters->stdout_fd))
                 fd = parameters->stdout_fd;
-        else if (path && (context->tty_path || is_terminal_input(context->std_input) ||
-                        is_terminal_output(context->std_output) || is_terminal_output(context->std_error))) {
+        else if (path && exec_context_has_tty(context)) {
                 fd = _fd = open_terminal(path, O_RDWR|O_NOCTTY|O_CLOEXEC|O_NONBLOCK);
                 if (fd < 0)
                         return (void) log_debug_errno(fd, "Failed to open terminal '%s', ignoring: %m", path);
@@ -180,7 +165,7 @@ void exec_context_tty_reset(const ExecContext *context, const ExecParameters *pa
         if (r < 0)
                 log_debug_errno(r, "Failed to configure TTY dimensions, ignoring: %m");
 
-        if (!sd_id128_is_null(invocation_id)) {
+        if (!sd_id128_is_null(invocation_id) && exec_context_shall_ansi_seq_reset(context)) {
                 sd_id128_t context_id;
 
                 r = osc_context_id_from_invocation_id(invocation_id, &context_id);
@@ -1000,9 +985,9 @@ static bool exec_context_may_touch_tty(const ExecContext *ec) {
         return ec->tty_reset ||
                 ec->tty_vhangup ||
                 ec->tty_vt_disallocate ||
-                is_terminal_input(ec->std_input) ||
-                is_terminal_output(ec->std_output) ||
-                is_terminal_output(ec->std_error);
+                exec_input_is_terminal(ec->std_input) ||
+                ec->std_output == EXEC_OUTPUT_TTY ||
+                ec->std_error == EXEC_OUTPUT_TTY;
 }
 
 bool exec_context_may_touch_console(const ExecContext *ec) {
@@ -1024,6 +1009,9 @@ bool exec_context_shall_ansi_seq_reset(const ExecContext *c) {
         if (!c->tty_reset)
                 return false;
 
+        /* FIXME:
+         * On invocation, we generate $TERM based on settings for StandardOutput= and friends and the kernel
+         * command line options, or propagate $TERM from the service manager. See setup_term_environment(). */
         return !streq_ptr(strv_env_get(c->environment, "TERM"), "dumb");
 }
 

src/core/execute.h

@@ -458,6 +458,36 @@ typedef struct ExecParameters {
                 .pidref_transport_fd    = -EBADF, \
         }
 
+static inline bool exec_input_is_terminal(ExecInput i) {
+        return IN_SET(i,
+                      EXEC_INPUT_TTY,
+                      EXEC_INPUT_TTY_FORCE,
+                      EXEC_INPUT_TTY_FAIL);
+}
+
+static inline bool exec_output_is_terminal(ExecOutput o) {
+        return IN_SET(o,
+                      EXEC_OUTPUT_TTY,
+                      EXEC_OUTPUT_KMSG_AND_CONSOLE,
+                      EXEC_OUTPUT_JOURNAL_AND_CONSOLE);
+}
+
+static inline bool exec_output_is_kmsg(ExecOutput o) {
+        return IN_SET(o,
+                      EXEC_OUTPUT_KMSG,
+                      EXEC_OUTPUT_KMSG_AND_CONSOLE);
+}
+
+static inline bool exec_context_has_tty(const ExecContext *context) {
+        assert(context);
+
+        return
+                context->tty_path ||
+                exec_input_is_terminal(context->std_input) ||
+                context->std_output == EXEC_OUTPUT_TTY ||
+                context->std_error == EXEC_OUTPUT_TTY;
+}
+
 int exec_spawn(
                 Unit *unit,
                 ExecCommand *command,

src/core/service.c

@@ -1418,7 +1418,8 @@ static int service_coldplug(Unit *u) {
                     SERVICE_DEAD_RESOURCES_PINNED))
                 (void) unit_setup_exec_runtime(u);
 
-        if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, SERVICE_MOUNTING))
+        if (IN_SET(s->deserialized_state, SERVICE_START_POST, SERVICE_RUNNING, SERVICE_RELOAD, SERVICE_RELOAD_SIGNAL, SERVICE_RELOAD_NOTIFY, SERVICE_REFRESH_EXTENSIONS, SERVICE_MOUNTING) &&
+            freezer_state_finish(u->freezer_state) == FREEZER_RUNNING)
                 service_start_watchdog(s);
 
         if (UNIT_ISSET(s->accept_socket)) {

src/import/import-compress.c

@@ -4,6 +4,7 @@
 #include <string.h>
 
 #include "import-compress.h"
+#include "log.h"
 #include "string-table.h"
 
 void import_compress_free(ImportCompress *c) {
@@ -104,6 +105,7 @@ int import_uncompress_detect(ImportCompress *c, const void *data, size_t size) {
 
         c->encoding = false;
 
+        log_debug("Detected compression type: %s", import_compress_type_to_string(c->type));
         return 1;
 }
 
@@ -589,15 +591,15 @@ int import_compress_finish(ImportCompress *c, void **buffer, size_t *buffer_size
 }
 
 static const char* const import_compress_type_table[_IMPORT_COMPRESS_TYPE_MAX] = {
-        [IMPORT_COMPRESS_UNKNOWN] = "unknown",
+        [IMPORT_COMPRESS_UNKNOWN]      = "unknown",
         [IMPORT_COMPRESS_UNCOMPRESSED] = "uncompressed",
-        [IMPORT_COMPRESS_XZ] = "xz",
-        [IMPORT_COMPRESS_GZIP] = "gzip",
+        [IMPORT_COMPRESS_XZ]           = "xz",
+        [IMPORT_COMPRESS_GZIP]         = "gzip",
 #if HAVE_BZIP2
-        [IMPORT_COMPRESS_BZIP2] = "bzip2",
+        [IMPORT_COMPRESS_BZIP2]        = "bzip2",
 #endif
 #if HAVE_ZSTD
-        [IMPORT_COMPRESS_ZSTD] = "zstd",
+        [IMPORT_COMPRESS_ZSTD]         = "zstd",
 #endif
 };
 

src/import/importd.c

@@ -721,7 +721,7 @@ static int manager_new(Manager **ret) {
 
         r = notify_socket_prepare(
                         m->event,
-                        SD_EVENT_PRIORITY_NORMAL,
+                        SD_EVENT_PRIORITY_NORMAL - 1, /* Make this processed before SIGCHLD. */
                         manager_on_notify,
                         m,
                         &m->notify_socket_path);

src/machine/machinectl.c

@@ -1400,18 +1400,25 @@ static int shell_machine(int argc, char *argv[], void *userdata) {
                 return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
                                        "Shell only supported on local machines.");
 
-        /* Pass $TERM & Co. to shell session, if not explicitly specified. */
-        FOREACH_STRING(v, "TERM=", "COLORTERM=", "NO_COLOR=") {
-                if (strv_find_prefix(arg_setenv, v))
-                        continue;
+        if (terminal_is_dumb()) {
+                /* Set TERM=dumb if we are running on a dumb terminal or with a pipe.
+                 * Otherwise, we will get unwanted OSC sequences. */
+                if (!strv_find_prefix(arg_setenv, "TERM="))
+                        if (strv_extend(&arg_setenv, "TERM=dumb") < 0)
+                                return log_oom();
+        } else
+                /* Pass $TERM & Co. to shell session, if not explicitly specified. */
+                FOREACH_STRING(v, "TERM=", "COLORTERM=", "NO_COLOR=") {
+                        if (strv_find_prefix(arg_setenv, v))
+                                continue;
 
-                const char *t = strv_find_prefix(environ, v);
-                if (!t)
-                        continue;
+                        const char *t = strv_find_prefix(environ, v);
+                        if (!t)
+                                continue;
 
-                if (strv_extend(&arg_setenv, t) < 0)
-                        return log_oom();
-        }
+                        if (strv_extend(&arg_setenv, t) < 0)
+                                return log_oom();
+                }
 
         (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
 

src/nspawn/nspawn.c

@@ -3551,7 +3551,7 @@ static int inner_child(
         envp[n_env++] = strjoina("container=", arg_container_service_name);
 
         /* Propagate $TERM & Co. unless we are invoked in pipe mode and stdin/stdout/stderr don't refer to a TTY */
-        if (arg_console_mode != CONSOLE_PIPE || on_tty()) {
+        if (arg_console_mode != CONSOLE_PIPE && !terminal_is_dumb())
                 FOREACH_STRING(v, "TERM=", "COLORTERM=", "NO_COLOR=") {
                         char *t = strv_find_prefix(environ, v);
                         if (!t)
@@ -3559,7 +3559,7 @@ static int inner_child(
 
                         envp[n_env++] = t;
                 }
-        } else
+        else
                 envp[n_env++] = (char*) "TERM=dumb";
 
         if (home || !uid_is_valid(arg_uid) || arg_uid == 0)

src/shared/pretty-print.c

@@ -530,6 +530,9 @@ bool shall_tint_background(void) {
 }
 
 void draw_progress_bar_unbuffered(const char *prefix, double percentage) {
+        if (!on_tty())
+                return;
+
         fputc('\r', stderr);
         if (prefix) {
                 fputs(prefix, stderr);
@@ -593,6 +596,9 @@ void draw_progress_bar_unbuffered(const char *prefix, double percentage) {
 }
 
 void clear_progress_bar_unbuffered(const char *prefix) {
+        if (!on_tty())
+                return;
+
         fputc('\r', stderr);
 
         if (terminal_is_dumb())
@@ -609,6 +615,9 @@ void clear_progress_bar_unbuffered(const char *prefix) {
 }
 
 void draw_progress_bar(const char *prefix, double percentage) {
+        if (!on_tty())
+                return;
+
         /* We are going output a bunch of small strings that shall appear as a single line to STDERR which is
          * unbuffered by default. Let's temporarily turn on full buffering, so that this is passed to the tty
          * as a single buffer, to make things more efficient. */
@@ -621,6 +630,9 @@ int draw_progress_barf(double percentage, const char *prefixf, ...) {
         va_list ap;
         int r;
 
+        if (!on_tty())
+                return 0;
+
         va_start(ap, prefixf);
         r = vasprintf(&s, prefixf, ap);
         va_end(ap);
@@ -633,6 +645,9 @@ int draw_progress_barf(double percentage, const char *prefixf, ...) {
 }
 
 void clear_progress_bar(const char *prefix) {
+        if (!on_tty())
+                return;
+
         WITH_BUFFERED_STDERR;
         clear_progress_bar_unbuffered(prefix);
 }

src/sysupdate/sysupdated.c

@@ -1746,7 +1746,7 @@ static int manager_new(Manager **ret) {
 
         r = notify_socket_prepare(
                         m->event,
-                        SD_EVENT_PRIORITY_NORMAL,
+                        SD_EVENT_PRIORITY_NORMAL - 1, /* Make this processed before SIGCHLD. */
                         manager_on_notify,
                         m,
                         &m->notify_socket_path);

src/ukify/ukify.py

@@ -1023,6 +1023,19 @@ def pe_add_sections(opts: UkifyConfig, uki: UKI, output: str) -> None:
         if section.name == '.linux':
             # Old kernels that use EFI handover protocol will be executed inline.
             new_section.IMAGE_SCN_CNT_CODE = True
+
+            # Check if the kernel PE has the NX_COMPAT flag set, if not strip it from the UKI as they need
+            # to have the same value, otherwise when firmwares start enforcing it, booting will fail.
+            # https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/
+            # https://www.kraxel.org/blog/2023/12/uefi-nx-linux-boot/
+            try:
+                inner_pe = pefile.PE(data=data, fast_load=True)
+                nxbit = pefile.DLL_CHARACTERISTICS['IMAGE_DLLCHARACTERISTICS_NX_COMPAT']
+                if not inner_pe.OPTIONAL_HEADER.DllCharacteristics & nxbit:
+                    pe.OPTIONAL_HEADER.DllCharacteristics &= ~nxbit
+            except pefile.PEFormatError:
+                # Unit tests build images with bogus data
+                print(f'{section.name} in {uki.executable} is not a valid PE, ignoring', file=sys.stderr)
         else:
             new_section.IMAGE_SCN_CNT_INITIALIZED_DATA = True
 

test/units/TEST-38-FREEZER.sh

@@ -69,42 +69,18 @@ dbus_thaw_unit() {
            "$1"
 }
 
-dbus_can_freeze() {
-    local name object_path suffix
-
-    suffix="${1##*.}"
-    name="${1%".$suffix"}"
-    object_path="/org/freedesktop/systemd1/unit/${name//-/_2d}_2e${suffix}"
-
-    busctl get-property \
-           org.freedesktop.systemd1 \
-           "${object_path}" \
-           org.freedesktop.systemd1.Unit \
-           CanFreeze
-}
-
 check_freezer_state() {
-    local name object_path suffix
+    local name state expected
 
-    suffix="${1##*.}"
-    name="${1%".$suffix"}"
-    object_path="/org/freedesktop/systemd1/unit/${name//-/_2d}_2e${suffix}"
+    name="${1:?}"
+    expected="${2:?}"
 
-    for _ in {0..10}; do
-        state=$(busctl get-property \
-                       org.freedesktop.systemd1 \
-                       "${object_path}" \
-                       org.freedesktop.systemd1.Unit \
-                       FreezerState | cut -d " " -f2 | tr -d '"')
+    # Ignore the intermediate freezing & thawing states in case we check the unit state too quickly.
+    timeout 10 bash -c "while [[ \"\$(systemctl show \"$name\" --property FreezerState --value)\" =~ (freezing|thawing) ]]; do sleep .5; done"
 
-        # Ignore the intermediate freezing & thawing states in case we check
-        # the unit state too quickly
-        [[ "$state" =~ ^(freezing|thawing) ]] || break
-        sleep .5
-    done
-
-    [ "$state" = "$2" ] || {
-        echo "error: unexpected freezer state, expected: $2, actual: $state" >&2
+    state="$(systemctl show "$name" --property FreezerState --value)"
+    [[ "$state" = "$expected" ]] || {
+        echo "error: unexpected freezer state, expected: $expected, actual: $state" >&2
         exit 1
     }
 }
@@ -146,9 +122,8 @@ testcase_dbus_api() {
     check_cgroup_state "$unit" 0
     echo "[ OK ]"
 
-    echo -n "  - CanFreeze(): "
-    output=$(dbus_can_freeze "${unit}")
-    [ "$output" = "b true" ]
+    echo -n "  - CanFreeze: "
+    [[ "$(systemctl show "${unit}" --property=CanFreeze --value)" == 'yes' ]]
     echo "[ OK ]"
 
     echo
@@ -367,12 +342,36 @@ testcase_watchdog() {
         /bin/bash -c 'systemd-notify --ready; while true; do systemd-notify WATCHDOG=1; sleep 1; done'
 
     systemctl freeze "$unit"
-
     check_freezer_state "$unit" "frozen"
     sleep 6
     check_freezer_state "$unit" "frozen"
 
     systemctl thaw "$unit"
+    check_freezer_state "$unit" "running"
+    sleep 6
+    check_freezer_state "$unit" "running"
+    systemctl is-active "$unit"
+
+    systemctl freeze "$unit"
+    check_freezer_state "$unit" "frozen"
+    systemctl daemon-reload
+    sleep 6
+    check_freezer_state "$unit" "frozen"
+
+    systemctl thaw "$unit"
+    check_freezer_state "$unit" "running"
+    sleep 6
+    check_freezer_state "$unit" "running"
+    systemctl is-active "$unit"
+
+    systemctl freeze "$unit"
+    check_freezer_state "$unit" "frozen"
+    systemctl daemon-reexec
+    sleep 6
+    check_freezer_state "$unit" "frozen"
+
+    systemctl thaw "$unit"
+    check_freezer_state "$unit" "running"
     sleep 6
     check_freezer_state "$unit" "running"
     systemctl is-active "$unit"

test/units/TEST-72-SYSUPDATE.sh

@@ -54,15 +54,18 @@ at_exit() {
 trap at_exit EXIT
 
 update_checksums() {
-	(cd "$WORKDIR/source" && sha256sum uki* part* dir-*.tar.gz >SHA256SUMS)
+    (cd "$WORKDIR/source" && sha256sum uki* part* dir-*.tar.gz >SHA256SUMS)
 }
 
 new_version() {
     local sector_size="${1:?}"
     local version="${2:?}"
 
-    # Create a pair of random partition payloads, and compress one
-    dd if=/dev/urandom of="$WORKDIR/source/part1-$version.raw" bs="$sector_size" count=2048
+    # Create a pair of random partition payloads, and compress one.
+    # To make not the initial bytes of part1-xxx.raw accidentally match one of the compression header,
+    # let's make the first sector filled by zero.
+    dd if=/dev/zero of="$WORKDIR/source/part1-$version.raw" bs="$sector_size" count=1
+    dd if=/dev/urandom of="$WORKDIR/source/part1-$version.raw" bs="$sector_size" count=2047 conv=notrunc oflag=append
     dd if=/dev/urandom of="$WORKDIR/source/part2-$version.raw" bs="$sector_size" count=2048
     gzip -k -f "$WORKDIR/source/part2-$version.raw"
 
@@ -354,7 +357,7 @@ EOF
         updatectl check
         rm -r /run/sysupdate.test.d
     fi
-    
+
     # Create seventh version, and update through a file:// URL. This should be
     # almost as good as testing HTTP, but is simpler for us to set up. file:// is
     # abstracted in curl for us, and since our main goal is to test our own code