grub2/0248-Rework-even-more-of-efi-chainload-so-non-sb-cases-wo.patch

From b4f97fae3f20b5ba3675a063196db399d14894b2 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Fri, 10 Jun 2016 14:06:15 -0400
Subject: [PATCH] Rework even more of efi chainload so non-sb cases work right.

This ensures that if shim protocol is not loaded, or is loaded but shim
is disabled, we will fall back to a correct load method for the efi
chain loader.

Here's what I tested with this version:

results                             expected    actual
------------------------------------------------------------
sb + enabled + shim + fedora        success     success
sb + enabled + shim + win           success     success
sb + enabled + grub + fedora        fail        fail
sb + enabled + grub + win           fail        fail

sb + mokdisabled + shim + fedora    success     success
sb + mokdisabled + shim + win       success     success
sb + mokdisabled + grub + fedora    fail        fail
sb + mokdisabled + grub + win       fail        fail

sb disabled + shim + fedora         success     success*
sb disabled + shim + win            success     success*
sb disabled + grub + fedora         success     success
sb disabled + grub + win            success     success

nosb + shim + fedora                success     success*
nosb + shim + win                   success     success*
nosb + grub + fedora                success     success
nosb + grub + win                   success     success

* for some reason shim protocol is being installed in these cases, and I
  can't see why, but I think it may be this firmware build returning an
  erroneous value.  But this effectively falls back to the mokdisabled
  behavior, which works correctly, and the presence of the "grub" (i.e.
  no shim) tests effectively tests the desired behavior here.

Resolves: rhbz#1344512

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 grub-core/kern/efi/sb.c            |  14 +++--
 grub-core/loader/arm64/linux.c     |   4 +-
 grub-core/loader/efi/chainloader.c | 124 ++++++++++++++++++++++---------------
 grub-core/loader/efi/linux.c       |  13 ++--
 grub-core/loader/i386/efi/linux.c  |  10 ++-
 include/grub/efi/linux.h           |   2 +-
 6 files changed, 103 insertions(+), 64 deletions(-)

diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index a41b6c5b851..d74778b0cac 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -36,14 +36,20 @@ grub_efi_secure_boot (void)
   grub_efi_boolean_t ret = 0;
 
   secure_boot = grub_efi_get_variable("SecureBoot", &efi_var_guid, &datasize);
-
   if (datasize != 1 || !secure_boot)
-    goto out;
+    {
+      grub_dprintf ("secureboot", "No SecureBoot variable\n");
+      goto out;
+    }
+  grub_dprintf ("secureboot", "SecureBoot: %d\n", *secure_boot);
 
   setup_mode = grub_efi_get_variable("SetupMode", &efi_var_guid, &datasize);
-
   if (datasize != 1 || !setup_mode)
-    goto out;
+    {
+      grub_dprintf ("secureboot", "No SetupMode variable\n");
+      goto out;
+    }
+  grub_dprintf ("secureboot", "SetupMode: %d\n", *setup_mode);
 
   if (*secure_boot && !*setup_mode)
     ret = 1;
diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
index ab0d27ee4df..fcab2ed37e7 100644
--- a/grub-core/loader/arm64/linux.c
+++ b/grub-core/loader/arm64/linux.c
@@ -248,6 +248,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
   grub_file_t file = 0;
   struct linux_arm64_kernel_header lh;
   struct grub_arm64_linux_pe_header *pe;
+  int rc;
 
   grub_dl_ref (my_mod);
 
@@ -292,7 +293,8 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
 
   grub_dprintf ("linux", "kernel @ %p\n", kernel_addr);
 
-  if (!grub_linuxefi_secure_validate (kernel_addr, kernel_size))
+  rc = grub_linuxefi_secure_validate (kernel_addr, kernel_size);
+  if (rc < 0)
     {
       grub_error (GRUB_ERR_INVALID_COMMAND, N_("%s has invalid signature"), argv[0]);
       goto fail;
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index 4b77a7d5adb..b977c7b5573 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -182,7 +182,6 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
   /* Fill the file path for the directory.  */
   d = (grub_efi_device_path_t *) ((char *) file_path
 				  + ((char *) d - (char *) dp));
-  grub_efi_print_device_path (d);
   copy_file_path ((grub_efi_file_path_device_path_t *) d,
 		  dir_start, dir_end - dir_start);
 
@@ -252,10 +251,9 @@ read_header (void *data, grub_efi_uint32_t size,
   grub_efi_status_t status;
 
   shim_lock = grub_efi_locate_protocol (&guid, NULL);
-
   if (!shim_lock)
     {
-      grub_error (GRUB_ERR_BAD_ARGUMENT, "no shim lock protocol");
+      grub_dprintf ("chain", "no shim lock protocol");
       return 0;
     }
 
@@ -280,7 +278,7 @@ read_header (void *data, grub_efi_uint32_t size,
       break;
     }
 
-  return 0;
+  return -1;
 }
 
 static void*
@@ -381,7 +379,7 @@ relocate_coff (pe_coff_loader_image_context_t *context,
       return GRUB_EFI_UNSUPPORTED;
     }
 
-  adjust = (grub_uint64_t)data - context->image_address;
+  adjust = (grub_uint64_t)(grub_efi_uintn_t)data - context->image_address;
   if (adjust == 0)
     return GRUB_EFI_SUCCESS;
 
@@ -514,18 +512,25 @@ handle_image (void *data, grub_efi_uint32_t datasize)
   grub_uint32_t section_alignment;
   grub_uint32_t buffer_size;
   int found_entry_point = 0;
+  int rc;
 
   b = grub_efi_system_table->boot_services;
 
-  if (read_header (data, datasize, &context))
-    {
-      grub_dprintf ("chain", "Succeed to read header\n");
-    }
-  else
+  rc = read_header (data, datasize, &context);
+  if (rc < 0)
     {
       grub_dprintf ("chain", "Failed to read header\n");
       goto error_exit;
     }
+  else if (rc == 0)
+    {
+      grub_dprintf ("chain", "Secure Boot is not enabled\n");
+      return 0;
+    }
+  else
+    {
+      grub_dprintf ("chain", "Header read without error\n");
+    }
 
   /*
    * The spec says, uselessly, of SectionAlignment:
@@ -547,7 +552,7 @@ handle_image (void *data, grub_efi_uint32_t datasize)
     section_alignment = 4096;
 
   buffer_size = context.image_size + section_alignment;
-  grub_dprintf ("chain", "image size is %08lx, datasize is %08x\n",
+  grub_dprintf ("chain", "image size is %08"PRIxGRUB_UINT64_T", datasize is %08x\n",
 	       context.image_size, datasize);
 
   efi_status = efi_call_3 (b->allocate_pool, GRUB_EFI_LOADER_DATA,
@@ -580,7 +585,7 @@ handle_image (void *data, grub_efi_uint32_t datasize)
 
   char *reloc_base, *reloc_base_end;
   grub_dprintf ("chain", "reloc_dir: %p reloc_size: 0x%08x\n",
-		(void *)(unsigned long long)context.reloc_dir->rva,
+		(void *)(unsigned long)context.reloc_dir->rva,
 		context.reloc_dir->size);
   reloc_base = image_address (buffer_aligned, context.image_size,
 			      context.reloc_dir->rva);
@@ -796,10 +801,56 @@ grub_secureboot_chainloader_unload (void)
   return GRUB_ERR_NONE;
 }
 
+static grub_err_t
+grub_load_and_start_image(void *boot_image)
+{
+  grub_efi_boot_services_t *b;
+  grub_efi_status_t status;
+  grub_efi_loaded_image_t *loaded_image;
+
+  b = grub_efi_system_table->boot_services;
+
+  status = efi_call_6 (b->load_image, 0, grub_efi_image_handle, file_path,
+		       boot_image, fsize, &image_handle);
+  if (status != GRUB_EFI_SUCCESS)
+    {
+      if (status == GRUB_EFI_OUT_OF_RESOURCES)
+	grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of resources");
+      else
+	grub_error (GRUB_ERR_BAD_OS, "cannot load image");
+      return -1;
+    }
+
+  /* LoadImage does not set a device handler when the image is
+     loaded from memory, so it is necessary to set it explicitly here.
+     This is a mess.  */
+  loaded_image = grub_efi_get_loaded_image (image_handle);
+  if (! loaded_image)
+    {
+      grub_error (GRUB_ERR_BAD_OS, "no loaded image available");
+      return -1;
+    }
+  loaded_image->device_handle = dev_handle;
+
+  if (cmdline)
+    {
+      loaded_image->load_options = cmdline;
+      loaded_image->load_options_size = cmdline_len;
+    }
+
+  return 0;
+}
+
 static grub_err_t
 grub_secureboot_chainloader_boot (void)
 {
-  handle_image ((void *)address, fsize);
+  int rc;
+  rc = handle_image ((void *)(unsigned long)address, fsize);
+  if (rc == 0)
+    {
+      grub_load_and_start_image((void *)(unsigned long)address);
+    }
+
   grub_loader_unset ();
   return grub_errno;
 }
@@ -813,9 +864,9 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
   grub_efi_boot_services_t *b;
   grub_device_t dev = 0;
   grub_efi_device_path_t *dp = 0;
-  grub_efi_loaded_image_t *loaded_image;
   char *filename;
   void *boot_image = 0;
+  int rc;
 
   if (argc == 0)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -902,9 +953,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
   if (! file_path)
     goto fail;
 
-  grub_printf ("file path: ");
-  grub_efi_print_device_path (file_path);
-
   fsize = grub_file_size (file);
   if (!fsize)
     {
@@ -979,51 +1027,27 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
     }
 #endif
 
-  if (grub_linuxefi_secure_validate((void *)address, fsize))
+  rc = grub_linuxefi_secure_validate((void *)(unsigned long)address, fsize);
+  grub_dprintf ("chain", "linuxefi_secure_validate: %d\n", rc);
+  if (rc > 0)
     {
       grub_file_close (file);
       grub_loader_set (grub_secureboot_chainloader_boot,
 		       grub_secureboot_chainloader_unload, 0);
       return 0;
     }
-
-  status = efi_call_6 (b->load_image, 0, grub_efi_image_handle, file_path,
-		       boot_image, fsize, &image_handle);
-  if (status != GRUB_EFI_SUCCESS)
+  else if (rc == 0)
     {
-      if (status == GRUB_EFI_OUT_OF_RESOURCES)
-	grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of resources");
-      else
-	grub_error (GRUB_ERR_BAD_OS, "cannot load image");
-
-      goto fail;
-    }
+      grub_load_and_start_image(boot_image);
+      grub_file_close (file);
+      grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
 
-  /* LoadImage does not set a device handler when the image is
-     loaded from memory, so it is necessary to set it explicitly here.
-     This is a mess.  */
-  loaded_image = grub_efi_get_loaded_image (image_handle);
-  if (! loaded_image)
-    {
-      grub_error (GRUB_ERR_BAD_OS, "no loaded image available");
-      goto fail;
-    }
-  loaded_image->device_handle = dev_handle;
-
-  if (cmdline)
-    {
-      loaded_image->load_options = cmdline;
-      loaded_image->load_options_size = cmdline_len;
+      return 0;
     }
 
   grub_file_close (file);
-  grub_device_close (dev);
-
-  grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
-  return 0;
-
- fail:
 
+fail:
   if (dev)
     grub_device_close (dev);
 
diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
index aea378adf5c..8890bdf059a 100644
--- a/grub-core/loader/efi/linux.c
+++ b/grub-core/loader/efi/linux.c
@@ -33,21 +33,24 @@ struct grub_efi_shim_lock
 };
 typedef struct grub_efi_shim_lock grub_efi_shim_lock_t;
 
-grub_efi_boolean_t
+int
 grub_linuxefi_secure_validate (void *data, grub_uint32_t size)
 {
   grub_efi_guid_t guid = SHIM_LOCK_GUID;
   grub_efi_shim_lock_t *shim_lock;
+  grub_efi_status_t status;
 
   shim_lock = grub_efi_locate_protocol(&guid, NULL);
-
+  grub_dprintf ("secureboot", "shim_lock: %p\n", shim_lock);
   if (!shim_lock)
-    return 1;
+    return 0;
 
-  if (shim_lock->verify(data, size) == GRUB_EFI_SUCCESS)
+  status = shim_lock->verify(data, size);
+  grub_dprintf ("secureboot", "shim_lock->verify(): %ld\n", status);
+  if (status == GRUB_EFI_SUCCESS)
     return 1;
 
-  return 0;
+  return -1;
 }
 
 typedef void (*handover_func) (void *, grub_efi_system_table_t *, void *);
diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c
index 7ccf32d9d45..82f75b7f3ab 100644
--- a/grub-core/loader/i386/efi/linux.c
+++ b/grub-core/loader/i386/efi/linux.c
@@ -155,6 +155,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
   struct linux_kernel_header lh;
   grub_ssize_t len, start, filelen;
   void *kernel = NULL;
+  int rc;
 
   grub_dl_ref (my_mod);
 
@@ -180,13 +181,16 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
 
   if (grub_file_read (file, kernel, filelen) != filelen)
     {
-      grub_error (GRUB_ERR_FILE_READ_ERROR, N_("Can't read kernel %s"), argv[0]);
+      grub_error (GRUB_ERR_FILE_READ_ERROR, N_("Can't read kernel %s"),
+		  argv[0]);
       goto fail;
     }
 
-  if (! grub_linuxefi_secure_validate (kernel, filelen))
+  rc = grub_linuxefi_secure_validate (kernel, filelen);
+  if (rc < 0)
     {
-      grub_error (GRUB_ERR_INVALID_COMMAND, N_("%s has invalid signature"), argv[0]);
+      grub_error (GRUB_ERR_INVALID_COMMAND, N_("%s has invalid signature"),
+		  argv[0]);
       grub_free (kernel);
       goto fail;
     }
diff --git a/include/grub/efi/linux.h b/include/grub/efi/linux.h
index d9ede36773b..0033d9305a9 100644
--- a/include/grub/efi/linux.h
+++ b/include/grub/efi/linux.h
@@ -22,7 +22,7 @@
 #include <grub/err.h>
 #include <grub/symbol.h>
 
-grub_efi_boolean_t
+int
 EXPORT_FUNC(grub_linuxefi_secure_validate) (void *data, grub_uint32_t size);
 grub_err_t
 EXPORT_FUNC(grub_efi_linux_boot) (void *kernel_address, grub_off_t offset,