boot: move/adjust comments

The comment in linux_exec() was based on Lennart's comment in
https://github.com/systemd/systemd/pull/37372#discussion_r2142340582, but
shortened. The original wording is more direct and at least for me easier to
grok, so adjust the comment to be more verbose again.

Also, move the comment from shim_loader_available() to the place where it used.
This function is for checking if the new thing is available, no need to describe
the old thing there.

diff --git a/src/boot/linux.c b/src/boot/linux.c
index 0eb8f9913f210e1bd40af95d19d76214dc637d91..1e8fe15bd1b71598e9a61ce8405dc283cf770c72 100644 (file)
--- a/src/boot/linux.c
+++ b/src/boot/linux.c
@@ -106,22 +106,28 @@ EFI_STATUS linux_exec(
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Bad kernel image: %m");
 
-        EFI_LOADED_IMAGE_PROTOCOL* parent_loaded_image;
+        EFI_LOADED_IMAGE_PROTOCOL *parent_loaded_image;
         err = BS->HandleProtocol(
                         parent, MAKE_GUID_PTR(EFI_LOADED_IMAGE_PROTOCOL), (void **) &parent_loaded_image);
         if (err != EFI_SUCCESS)
                 return log_error_status(err, "Cannot get parent loaded image: %m");
 
-        /* If shim provides LoadImage, it comes from version 16.1 or later and does the following:
-         * - It keeps a database of all PE sections that it already authenticated.
-         * - shim's LoadImage always verifies PE images against denylists: dbx, mokx, sbat.
-         * - If the PE image was not authenticated as a PE section it will also:
-         *   + verify it against allowlists: db, mok
-         *   + measure it on PCR 4
+        /* If shim provides LoadImage, it comes from the new SHIM_IMAGE_LOADER interface added in shim 16,
+         * and implements the following:
+         * - shim hashes PE sections of PE binaries it authenticates and stores the hashes in a global
+         *   database.
+         * - shim's LoadImage always verifies PE images against denylists: DBX, MOKX, SBAT.
+         * - If the PE image was _not_ authenticated as a PE section it will also:
+         *   + verify it against allowlists: DB, MOK,
+         *   + measure it on PCR 4.
          *
-         * In our case, we are loading a PE section that was already authenticated as part of the UKI.
-         * So in contrast to a normal UEFI LoadImage, shim will verify extra denylists (mokx, sbat),
-         * while skipping all allowlists and measurements.
+         * (Compared to standard UEFI LoadImage(), the patched shim version of LoadImage() is both stricter —
+         * as it checks SBAT + MOKX for all PE payloads — and more relaxed — as it disables DB checks for PE
+         * payloads it has seen as part of another PE binary before.)
+         *
+         * In our case, we are loading a PE section that was already authenticated as part of the UKI. In
+         * contrast to a normal UEFI LoadImage, shim will verify extra denylists (MOKX, SBAT), but skip all
+         * allowlists and measurements.
          *
          * See https://github.com/rhboot/shim/blob/main/README.md#shim-loader-protocol
          */
@@ -187,8 +193,8 @@ EFI_STATUS linux_exec(
                 .FilePath = file_path,
                 .ImageBase = loaded_kernel,
                 .ImageSize = kernel_size_in_memory,
-                .ImageCodeType = /*EFI_LOADER_CODE*/1,
-                .ImageDataType = /*EFI_LOADER_DATA*/2,
+                .ImageCodeType = 1 /* EFI_LOADER_CODE */,
+                .ImageDataType = 2 /* EFI_LOADER_DATA */,
         };
 
         if (cmdline) {

diff --git a/src/boot/shim.c b/src/boot/shim.c
index c84e7fe589a5e700fe065999441ba58b00e97fbe..0168bb424d84d86f8f8ac180c6b5b4e8b410950a 100644 (file)
--- a/src/boot/shim.c
+++ b/src/boot/shim.c
@@ -42,11 +42,7 @@ bool shim_loaded(void) {
         return BS->LocateProtocol(MAKE_GUID_PTR(SHIM_LOCK), NULL, (void **) &shim_lock) == EFI_SUCCESS;
 }
 
-/* The shim lock protocol is for pre-v16 shim, where it was not hooked up to the BS->LoadImage() system
- * table and friends, and it has to be checked manually via the shim_validate() helper. If the shim image
- * loader protocol is available (shim v16 and newer), then it will have overridden BS->LoadImage() and
- * friends in the system table, so no specific helper is needed, and the standard BS->LoadImage() and
- * friends can be called instead. */
+/* Check if SHIM_IMAGE_LOADER is available, shim 16 or newer. */
 bool shim_loader_available(void) {
         void *shim_image_loader;
 
@@ -104,7 +100,15 @@ EFI_STATUS shim_load_image(
         assert(device_path);
         assert(ret_image);
 
+        /* The shim lock protocol is for pre-v16 shim, where it was not hooked up to the BS->LoadImage()
+         * system table and friends, and it has to be checked manually via the shim_validate() helper. If the
+         * shim image loader protocol is available (shim v16 and newer), then it will have overridden
+         * BS->LoadImage() and friends in the system table, so no specific helper is needed, and the standard
+         * BS->LoadImage() and friends can be called instead.
+         */
+
         // TODO: drop lock protocol and just use plain BS->LoadImage once Shim < 16 is no longer supported
+
         bool have_shim = shim_loaded() && !shim_loader_available();
 
         if (have_shim)