From 876ddd32b8c8c9be4e964862e608ea4b6bd37ff8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2023 07:57:43 -0500 Subject: [PATCH 1/4] zipl: Use O_CLOEXEC On general principle. --- src/libostree/ostree-bootloader-zipl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index f92cc61dcd..43a74cace5 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -142,7 +142,7 @@ _ostree_secure_boot_is_enabled (gboolean *out_enabled, GCancellable *cancellable // [ 0.023198] setup: 0000000000867000 - 0000000000868000 (not signed) // [ 0.023199] setup: 0000000000877000 - 0000000000878000 (not signed) // [ 0.023200] setup: 0000000000880000 - 0000000003f98000 (not signed) - fd = openat (AT_FDCWD, "/dev/kmsg", O_NONBLOCK | O_RDONLY); + fd = openat (AT_FDCWD, "/dev/kmsg", O_NONBLOCK | O_RDONLY | O_CLOEXEC); if (fd == -1) return glnx_throw_errno_prefix (error, "openat(/dev/kmsg)"); unsigned max_lines = 5; // no need to read dozens of messages, ours comes really early From e9a2a2cf25fc5070d6a0e74f86f45be2675daadd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2023 08:00:38 -0500 Subject: [PATCH 2/4] zipl: Fix error handling for read The return value is not errno. --- src/libostree/ostree-bootloader-zipl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 43a74cace5..d6fb7ebd3e 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -149,8 +149,8 @@ _ostree_secure_boot_is_enabled (gboolean *out_enabled, GCancellable *cancellable while (*out_enabled != TRUE && max_lines > 0) { char buf[1024]; - ssize_t len = read (fd, buf, sizeof (buf)); - if (len == -EAGAIN) + ssize_t len = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf))); + if (len < 0) break; *out_enabled = strstr (buf, "Secure-IPL enabled") != NULL; --max_lines; From 241597a8a5bbc4223d7739ffe191e5114af40caa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2023 08:03:01 -0500 Subject: [PATCH 3/4] zipl: NUL terminate buffer we're searching Found by a static analyzer. --- src/libostree/ostree-bootloader-zipl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index d6fb7ebd3e..0329eb658c 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -149,9 +149,10 @@ _ostree_secure_boot_is_enabled (gboolean *out_enabled, GCancellable *cancellable while (*out_enabled != TRUE && max_lines > 0) { char buf[1024]; - ssize_t len = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf))); + ssize_t len = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf) - 1)); if (len < 0) break; + buf[len] = '\0'; *out_enabled = strstr (buf, "Secure-IPL enabled") != NULL; --max_lines; } From ade0bd2693c9ffa1f421d5758b16314b895f981f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2023 08:06:57 -0500 Subject: [PATCH 4/4] zipl: Convert to a data input stream This high level reader API avoids all the bugs that were found in previous patches. --- src/libostree/ostree-bootloader-zipl.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libostree/ostree-bootloader-zipl.c b/src/libostree/ostree-bootloader-zipl.c index 0329eb658c..2804ed2644 100644 --- a/src/libostree/ostree-bootloader-zipl.c +++ b/src/libostree/ostree-bootloader-zipl.c @@ -22,6 +22,7 @@ #include "ostree-libarchive-private.h" #include "ostree-sysroot-private.h" #include "otutil.h" +#include #include #include #include @@ -145,15 +146,14 @@ _ostree_secure_boot_is_enabled (gboolean *out_enabled, GCancellable *cancellable fd = openat (AT_FDCWD, "/dev/kmsg", O_NONBLOCK | O_RDONLY | O_CLOEXEC); if (fd == -1) return glnx_throw_errno_prefix (error, "openat(/dev/kmsg)"); - unsigned max_lines = 5; // no need to read dozens of messages, ours comes really early + g_autoptr (GInputStream) istream = g_unix_input_stream_new (g_steal_fd (&fd), TRUE); + g_autoptr (GDataInputStream) stream = g_data_input_stream_new (istream); + unsigned int max_lines = 5; // no need to read dozens of messages, ours comes really early while (*out_enabled != TRUE && max_lines > 0) { - char buf[1024]; - ssize_t len = TEMP_FAILURE_RETRY (read (fd, buf, sizeof (buf) - 1)); - if (len < 0) - break; - buf[len] = '\0'; - *out_enabled = strstr (buf, "Secure-IPL enabled") != NULL; + gsize len = 0; + g_autofree char *line = g_data_input_stream_read_line (stream, &len, NULL, error); + *out_enabled = strstr (line, "Secure-IPL enabled") != NULL; --max_lines; } ot_journal_print (LOG_INFO, "s390x: kmsg: Secure Boot enabled: %d", *out_enabled);