From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Chris Coulson <chris.coulson@canonical.com>

Date: Fri, 29 Apr 2022 21:16:02 +0100

Subject: [PATCH] commands/boot: Add API to pass context to loader

Loaders rely on global variables for saving context which is consumed

in the boot hook and freed in the unload hook. In the case where a loader

command is executed twice, calling grub_loader_set a second time executes

the unload hook, but in some cases this runs when the loader's global

context has already been updated, resulting in the updated context being

freed and potential use-after-free bugs when the boot hook is subsequently

called.

This adds a new API (grub_loader_set_ex) which allows a loader to specify

context that is passed to its boot and unload hooks. This is an alternative

to requiring that loaders call grub_loader_unset before mutating their

global context.

Signed-off-by: Chris Coulson <chris.coulson@canonical.com>

(cherry picked from commit 4322a64dde7e8fedb58e50b79408667129d45dd3)

---

 grub-core/commands/boot.c | 66 +++++++++++++++++++++++++++++++++++++++++------

 include/grub/loader.h     |  5 ++++

 2 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/grub-core/commands/boot.c b/grub-core/commands/boot.c

index bbca81e947..53691a62d9 100644

--- a/grub-core/commands/boot.c

+++ b/grub-core/commands/boot.c

@@ -27,10 +27,20 @@

 GRUB_MOD_LICENSE ("GPLv3+");

-static grub_err_t (*grub_loader_boot_func) (void);

-static grub_err_t (*grub_loader_unload_func) (void);

+static grub_err_t (*grub_loader_boot_func) (void *);

+static grub_err_t (*grub_loader_unload_func) (void *);

+static void *grub_loader_context;

 static int grub_loader_flags;

+struct grub_simple_loader_hooks

+{

+  grub_err_t (*boot) (void);

+  grub_err_t (*unload) (void);

+};

+

+/* Don't heap allocate this to avoid making grub_loader_set fallible. */

+static struct grub_simple_loader_hooks simple_loader_hooks;

+

 struct grub_preboot

 {

   grub_err_t (*preboot_func) (int);

@@ -44,6 +54,29 @@ static int grub_loader_loaded;

 static struct grub_preboot *preboots_head = 0,

   *preboots_tail = 0;

+static grub_err_t

+grub_simple_boot_hook (void *context)

+{

+  struct grub_simple_loader_hooks *hooks;

+

+  hooks = (struct grub_simple_loader_hooks *) context;

+  return hooks->boot ();

+}

+

+static grub_err_t

+grub_simple_unload_hook (void *context)

+{

+  struct grub_simple_loader_hooks *hooks;

+  grub_err_t ret;

+

+  hooks = (struct grub_simple_loader_hooks *) context;

+

+  ret = hooks->unload ();

+  grub_memset (hooks, 0, sizeof (*hooks));

+

+  return ret;

+}

+

 int

 grub_loader_is_loaded (void)

 {

@@ -110,28 +143,45 @@ grub_loader_unregister_preboot_hook (struct grub_preboot *hnd)

 }

 void

-grub_loader_set (grub_err_t (*boot) (void),

-		 grub_err_t (*unload) (void),

-		 int flags)

+grub_loader_set_ex (grub_err_t (*boot) (void *),

+		    grub_err_t (*unload) (void *),

+		    void *context,

+		    int flags)

 {

   if (grub_loader_loaded && grub_loader_unload_func)

-    grub_loader_unload_func ();

+    grub_loader_unload_func (grub_loader_context);

   grub_loader_boot_func = boot;

   grub_loader_unload_func = unload;

+  grub_loader_context = context;

   grub_loader_flags = flags;

   grub_loader_loaded = 1;

 }

+void

+grub_loader_set (grub_err_t (*boot) (void),

+		 grub_err_t (*unload) (void),

+		 int flags)

+{

+  grub_loader_set_ex (grub_simple_boot_hook,

+		      grub_simple_unload_hook,

+		      &simple_loader_hooks,

+		      flags);

+

+  simple_loader_hooks.boot = boot;

+  simple_loader_hooks.unload = unload;

+}

+

 void

 grub_loader_unset(void)

 {

   if (grub_loader_loaded && grub_loader_unload_func)

-    grub_loader_unload_func ();

+    grub_loader_unload_func (grub_loader_context);

   grub_loader_boot_func = 0;

   grub_loader_unload_func = 0;

+  grub_loader_context = 0;

   grub_loader_loaded = 0;

 }

@@ -158,7 +208,7 @@ grub_loader_boot (void)

 	  return err;

 	}

     }

-  err = (grub_loader_boot_func) ();

+  err = (grub_loader_boot_func) (grub_loader_context);

   for (cur = preboots_tail; cur; cur = cur->prev)

     if (! err)

diff --git a/include/grub/loader.h b/include/grub/loader.h

index b208642821..1846fa6c5f 100644

--- a/include/grub/loader.h

+++ b/include/grub/loader.h

@@ -40,6 +40,11 @@ void EXPORT_FUNC (grub_loader_set) (grub_err_t (*boot) (void),

 				    grub_err_t (*unload) (void),

 				    int flags);

+void EXPORT_FUNC (grub_loader_set_ex) (grub_err_t (*boot) (void *),

+				       grub_err_t (*unload) (void *),

+				       void *context,

+				       int flags);

+

 /* Unset current loader, if any.  */

 void EXPORT_FUNC (grub_loader_unset) (void);