Merge branch 'c8' into a8

This commit is contained in:
eabdullin 2022-06-17 10:36:32 +03:00
commit 7c8932dcd0
53 changed files with 5457 additions and 9 deletions

View File

@ -0,0 +1,72 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Thu, 28 Apr 2022 21:53:36 +0100
Subject: [PATCH] loader/efi/chainloader: grub_load_and_start_image doesn't
load and start
grub_load_and_start_image only loads an image - it still requires the
caller to start it. This renames it to grub_load_image.
It's called from 2 places:
- grub_cmd_chainloader when not using the shim protocol.
- grub_secureboot_chainloader_boot if handle_image returns an error.
In this case, the image is loaded and then nothing else happens which
seems strange. I assume the intention is that it falls back to LoadImage
and StartImage if handle_image fails, so I've made it do that.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit b4d70820a65c00561045856b7b8355461a9545f6)
(cherry picked from commit 05b16a6be50b1910609740a66b561276fa490538)
(cherry picked from commit 16486a34f3aa41a94e334e86db1a1e21e9b0a45f)
(cherry picked from commit 4a23f40cb6400d94621de688a7e79dfe124f5a63)
---
grub-core/loader/efi/chainloader.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index 29663f7180..d75d345003 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -835,7 +835,7 @@ grub_secureboot_chainloader_unload (void)
}
static grub_err_t
-grub_load_and_start_image(void *boot_image)
+grub_load_image(void *boot_image)
{
grub_efi_boot_services_t *b;
grub_efi_status_t status;
@@ -877,13 +877,23 @@ grub_load_and_start_image(void *boot_image)
static grub_err_t
grub_secureboot_chainloader_boot (void)
{
+ grub_efi_boot_services_t *b;
int rc;
+
rc = handle_image ((void *)(unsigned long)address, fsize);
if (rc == 0)
{
- grub_load_and_start_image((void *)(unsigned long)address);
+ /* We weren't able to attempt to execute the image, so fall back
+ * to LoadImage / StartImage.
+ */
+ rc = grub_load_image((void *)(unsigned long)address);
+ if (rc == 0)
+ grub_chainloader_boot ();
}
+ b = grub_efi_system_table->boot_services;
+ efi_call_1 (b->unload_image, image_handle);
+
grub_loader_unset ();
return grub_errno;
}
@@ -1072,7 +1082,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
}
else if (rc == 0)
{
- grub_load_and_start_image(boot_image);
+ grub_load_image(boot_image);
grub_file_close (file);
grub_device_close (dev);
grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);

View File

@ -0,0 +1,335 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Fri, 29 Apr 2022 21:13:08 +0100
Subject: [PATCH] loader/efi/chainloader: simplify the loader state
When not using the shim lock protocol, the chainloader command retains
the source buffer and device path passed to LoadImage, requiring the
unload hook passed to grub_loader_set to free them. It isn't required
to retain this state though - they aren't required by StartImage or
anything else in the boot hook, so clean them up before
grub_cmd_chainloader finishes.
This also wraps the loader state when using the shim lock protocol
inside a struct.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit fa39862933b3be1553a580a3a5c28073257d8046)
(cherry picked from commit 0333343ee99c4e88f062789263c94291c057251b)
[rharwood: verifying twice]
(cherry picked from commit 6080ad5d91d6a80d5f67c592dd33b6dd413e9453)
[rharwood: double frees and unintialized, context fuzz - orig_dp]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit b44b88ae45008611ec0469fb47139f4c0d1ee233)
---
grub-core/loader/efi/chainloader.c | 160 +++++++++++++++++++++++--------------
1 file changed, 102 insertions(+), 58 deletions(-)
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index d75d345003..afeb1fc97e 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -47,38 +47,21 @@ GRUB_MOD_LICENSE ("GPLv3+");
static grub_dl_t my_mod;
-static grub_efi_physical_address_t address;
-static grub_efi_uintn_t pages;
-static grub_ssize_t fsize;
-static grub_efi_device_path_t *file_path;
static grub_efi_handle_t image_handle;
-static grub_efi_char16_t *cmdline;
-static grub_ssize_t cmdline_len;
-static grub_efi_handle_t dev_handle;
-static grub_efi_status_t (*entry_point) (grub_efi_handle_t image_handle, grub_efi_system_table_t *system_table);
+struct grub_secureboot_chainloader_context {
+ grub_efi_physical_address_t address;
+ grub_efi_uintn_t pages;
+ grub_ssize_t fsize;
+ grub_efi_device_path_t *file_path;
+ grub_efi_char16_t *cmdline;
+ grub_ssize_t cmdline_len;
+ grub_efi_handle_t dev_handle;
+};
+static struct grub_secureboot_chainloader_context *sb_context;
static grub_err_t
-grub_chainloader_unload (void)
-{
- grub_efi_boot_services_t *b;
-
- b = grub_efi_system_table->boot_services;
- efi_call_1 (b->unload_image, image_handle);
- grub_efi_free_pages (address, pages);
-
- grub_free (file_path);
- grub_free (cmdline);
- cmdline = 0;
- file_path = 0;
- dev_handle = 0;
-
- grub_dl_unref (my_mod);
- return GRUB_ERR_NONE;
-}
-
-static grub_err_t
-grub_chainloader_boot (void)
+grub_start_image (grub_efi_handle_t handle)
{
grub_efi_boot_services_t *b;
grub_efi_status_t status;
@@ -86,7 +69,7 @@ grub_chainloader_boot (void)
grub_efi_char16_t *exit_data = NULL;
b = grub_efi_system_table->boot_services;
- status = efi_call_3 (b->start_image, image_handle, &exit_data_size, &exit_data);
+ status = efi_call_3 (b->start_image, handle, &exit_data_size, &exit_data);
if (status != GRUB_EFI_SUCCESS)
{
if (exit_data)
@@ -110,11 +93,37 @@ grub_chainloader_boot (void)
if (exit_data)
grub_efi_free_pool (exit_data);
- grub_loader_unset ();
-
return grub_errno;
}
+static grub_err_t
+grub_chainloader_unload (void)
+{
+ grub_efi_loaded_image_t *loaded_image;
+ grub_efi_boot_services_t *b;
+
+ loaded_image = grub_efi_get_loaded_image (image_handle);
+ if (loaded_image != NULL)
+ grub_free (loaded_image->load_options);
+
+ b = grub_efi_system_table->boot_services;
+ efi_call_1 (b->unload_image, image_handle);
+
+ grub_dl_unref (my_mod);
+ return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+grub_chainloader_boot (void)
+{
+ grub_err_t err;
+
+ err = grub_start_image (image_handle);
+
+ grub_loader_unset ();
+ return err;
+}
+
static grub_err_t
copy_file_path (grub_efi_file_path_device_path_t *fp,
const char *str, grub_efi_uint16_t len)
@@ -149,7 +158,7 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
char *dir_start;
char *dir_end;
grub_size_t size;
- grub_efi_device_path_t *d;
+ grub_efi_device_path_t *d, *file_path;
dir_start = grub_strchr (filename, ')');
if (! dir_start)
@@ -520,10 +529,12 @@ grub_efi_get_media_file_path (grub_efi_device_path_t *dp)
}
static grub_efi_boolean_t
-handle_image (void *data, grub_efi_uint32_t datasize)
+handle_image (struct grub_secureboot_chainloader_context *load_context)
{
grub_efi_loaded_image_t *li, li_bak;
grub_efi_status_t efi_status;
+ void *data = (void *)(unsigned long)load_context->address;
+ grub_efi_uint32_t datasize = load_context->fsize;
void *buffer = NULL;
char *buffer_aligned = NULL;
grub_efi_uint32_t i;
@@ -534,6 +545,7 @@ handle_image (void *data, grub_efi_uint32_t datasize)
grub_uint32_t buffer_size;
int found_entry_point = 0;
int rc;
+ grub_efi_status_t (*entry_point) (grub_efi_handle_t image_handle, grub_efi_system_table_t *system_table);
rc = read_header (data, datasize, &context);
if (rc < 0)
@@ -791,10 +803,10 @@ handle_image (void *data, grub_efi_uint32_t datasize)
grub_memcpy (&li_bak, li, sizeof (grub_efi_loaded_image_t));
li->image_base = buffer_aligned;
li->image_size = context.image_size;
- li->load_options = cmdline;
- li->load_options_size = cmdline_len;
- li->file_path = grub_efi_get_media_file_path (file_path);
- li->device_handle = dev_handle;
+ li->load_options = load_context->cmdline;
+ li->load_options_size = load_context->cmdline_len;
+ li->file_path = grub_efi_get_media_file_path (load_context->file_path);
+ li->device_handle = load_context->dev_handle;
if (!li->file_path)
{
grub_error (GRUB_ERR_UNKNOWN_DEVICE, "no matching file path found");
@@ -823,19 +835,22 @@ error_exit:
static grub_err_t
grub_secureboot_chainloader_unload (void)
{
- grub_efi_free_pages (address, pages);
- grub_free (file_path);
- grub_free (cmdline);
- cmdline = 0;
- file_path = 0;
- dev_handle = 0;
+ grub_efi_free_pages (sb_context->address, sb_context->pages);
+ grub_free (sb_context->file_path);
+ grub_free (sb_context->cmdline);
+ grub_free (sb_context);
+
+ sb_context = 0;
grub_dl_unref (my_mod);
return GRUB_ERR_NONE;
}
static grub_err_t
-grub_load_image(void *boot_image)
+grub_load_image(grub_efi_device_path_t *file_path, void *boot_image,
+ grub_efi_uintn_t image_size, grub_efi_handle_t dev_handle,
+ grub_efi_char16_t *cmdline, grub_ssize_t cmdline_len,
+ grub_efi_handle_t *image_handle_out)
{
grub_efi_boot_services_t *b;
grub_efi_status_t status;
@@ -844,7 +859,7 @@ grub_load_image(void *boot_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);
+ boot_image, image_size, image_handle_out);
if (status != GRUB_EFI_SUCCESS)
{
if (status == GRUB_EFI_OUT_OF_RESOURCES)
@@ -857,7 +872,7 @@ grub_load_image(void *boot_image)
/* 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);
+ loaded_image = grub_efi_get_loaded_image (*image_handle_out);
if (! loaded_image)
{
grub_error (GRUB_ERR_BAD_OS, "no loaded image available");
@@ -879,20 +894,25 @@ grub_secureboot_chainloader_boot (void)
{
grub_efi_boot_services_t *b;
int rc;
+ grub_efi_handle_t handle = 0;
- rc = handle_image ((void *)(unsigned long)address, fsize);
+ rc = handle_image (sb_context);
if (rc == 0)
{
/* We weren't able to attempt to execute the image, so fall back
* to LoadImage / StartImage.
*/
- rc = grub_load_image((void *)(unsigned long)address);
+ rc = grub_load_image(sb_context->file_path,
+ (void *)(unsigned long)sb_context->address,
+ sb_context->fsize, sb_context->dev_handle,
+ sb_context->cmdline, sb_context->cmdline_len,
+ &handle);
if (rc == 0)
- grub_chainloader_boot ();
+ grub_start_image (handle);
}
b = grub_efi_system_table->boot_services;
- efi_call_1 (b->unload_image, image_handle);
+ efi_call_1 (b->unload_image, handle);
grub_loader_unset ();
return grub_errno;
@@ -906,10 +926,16 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
grub_efi_status_t status;
grub_efi_boot_services_t *b;
grub_device_t dev = 0;
- grub_efi_device_path_t *dp = 0;
+ grub_efi_device_path_t *dp = 0, *file_path = 0;
char *filename;
void *boot_image = 0;
int rc;
+ grub_efi_physical_address_t address = 0;
+ grub_ssize_t fsize;
+ grub_efi_uintn_t pages = 0;
+ grub_efi_char16_t *cmdline = 0;
+ grub_ssize_t cmdline_len = 0;
+ grub_efi_handle_t dev_handle = 0;
if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -917,12 +943,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
grub_dl_ref (my_mod);
- /* Initialize some global variables. */
- address = 0;
- image_handle = 0;
- file_path = 0;
- dev_handle = 0;
-
b = grub_efi_system_table->boot_services;
if (argc > 1)
@@ -1074,17 +1094,35 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
grub_dprintf ("chain", "linuxefi_secure_validate: %d\n", rc);
if (rc > 0)
{
+ sb_context = grub_malloc (sizeof (*sb_context));
+ if (sb_context == NULL)
+ goto fail;
+ sb_context->address = address;
+ sb_context->fsize = fsize;
+ sb_context->pages = pages;
+ sb_context->file_path = file_path;
+ sb_context->cmdline = cmdline;
+ sb_context->cmdline_len = cmdline_len;
+ sb_context->dev_handle = dev_handle;
+
grub_file_close (file);
grub_device_close (dev);
+
grub_loader_set (grub_secureboot_chainloader_boot,
grub_secureboot_chainloader_unload, 0);
return 0;
}
else if (rc == 0)
{
- grub_load_image(boot_image);
+ grub_load_image(file_path, boot_image, fsize, dev_handle, cmdline,
+ cmdline_len, &image_handle);
grub_file_close (file);
grub_device_close (dev);
+
+ /* We're finished with the source image buffer and file path now */
+ efi_call_2 (b->free_pages, address, pages);
+ grub_free (file_path);
+
grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
return 0;
@@ -1106,6 +1144,12 @@ fail:
if (cmdline)
grub_free (cmdline);
+ if (image_handle != 0)
+ {
+ efi_call_1 (b->unload_image, image_handle);
+ image_handle = 0;
+ }
+
grub_dl_unref (my_mod);
return grub_errno;

View File

@ -0,0 +1,161 @@
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)
(cherry picked from commit 937ad0e2159b6b8cb0d2ce3515da3a8b797c7927)
(cherry picked from commit 873038ae7048f6cae8a3ebb2f97a8d361a080e13)
(cherry picked from commit 7eefe9ba7e8f1557705f0f854ab7a3014d6cb5e2)
---
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);

View File

@ -0,0 +1,150 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Fri, 29 Apr 2022 21:30:56 +0100
Subject: [PATCH] loader/efi/chainloader: Use grub_loader_set_ex
This ports the EFI chainloader to use grub_loader_set_ex in order to fix
a use-after-free bug that occurs when grub_cmd_chainloader is executed
more than once before a boot attempt is performed.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit 4b7f0402b7cb0f67a93be736f2b75b818d7f44c9)
(cherry picked from commit fc1a79bf0e0bc019362ace46d908a92b48dcd55b)
(cherry picked from commit f5b653dfe00271384ff7fbd82db926ab95dbd80e)
(cherry picked from commit 535a9d787f71ed6eb43e7c3a136a149684ec62ea)
[rharwood: context sludge from previous commit]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
grub-core/loader/efi/chainloader.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
index afeb1fc97e..720f6181e5 100644
--- a/grub-core/loader/efi/chainloader.c
+++ b/grub-core/loader/efi/chainloader.c
@@ -47,8 +47,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
static grub_dl_t my_mod;
-static grub_efi_handle_t image_handle;
-
struct grub_secureboot_chainloader_context {
grub_efi_physical_address_t address;
grub_efi_uintn_t pages;
@@ -58,7 +56,6 @@ struct grub_secureboot_chainloader_context {
grub_ssize_t cmdline_len;
grub_efi_handle_t dev_handle;
};
-static struct grub_secureboot_chainloader_context *sb_context;
static grub_err_t
grub_start_image (grub_efi_handle_t handle)
@@ -97,11 +94,14 @@ grub_start_image (grub_efi_handle_t handle)
}
static grub_err_t
-grub_chainloader_unload (void)
+grub_chainloader_unload (void *context)
{
+ grub_efi_handle_t image_handle;
grub_efi_loaded_image_t *loaded_image;
grub_efi_boot_services_t *b;
+ image_handle = (grub_efi_handle_t) context;
+
loaded_image = grub_efi_get_loaded_image (image_handle);
if (loaded_image != NULL)
grub_free (loaded_image->load_options);
@@ -114,10 +114,12 @@ grub_chainloader_unload (void)
}
static grub_err_t
-grub_chainloader_boot (void)
+grub_chainloader_boot (void *context)
{
+ grub_efi_handle_t image_handle;
grub_err_t err;
+ image_handle = (grub_efi_handle_t) context;
err = grub_start_image (image_handle);
grub_loader_unset ();
@@ -833,15 +835,17 @@ error_exit:
}
static grub_err_t
-grub_secureboot_chainloader_unload (void)
+grub_secureboot_chainloader_unload (void *context)
{
+ struct grub_secureboot_chainloader_context *sb_context;
+
+ sb_context = (struct grub_secureboot_chainloader_context *) context;
+
grub_efi_free_pages (sb_context->address, sb_context->pages);
grub_free (sb_context->file_path);
grub_free (sb_context->cmdline);
grub_free (sb_context);
- sb_context = 0;
-
grub_dl_unref (my_mod);
return GRUB_ERR_NONE;
}
@@ -890,12 +894,15 @@ grub_load_image(grub_efi_device_path_t *file_path, void *boot_image,
}
static grub_err_t
-grub_secureboot_chainloader_boot (void)
+grub_secureboot_chainloader_boot (void *context)
{
+ struct grub_secureboot_chainloader_context *sb_context;
grub_efi_boot_services_t *b;
int rc;
grub_efi_handle_t handle = 0;
+ sb_context = (struct grub_secureboot_chainloader_context *) context;
+
rc = handle_image (sb_context);
if (rc == 0)
{
@@ -936,6 +943,8 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
grub_efi_char16_t *cmdline = 0;
grub_ssize_t cmdline_len = 0;
grub_efi_handle_t dev_handle = 0;
+ grub_efi_handle_t image_handle = 0;
+ struct grub_secureboot_chainloader_context *sb_context = 0;
if (argc == 0)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
@@ -1108,8 +1117,8 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
grub_file_close (file);
grub_device_close (dev);
- grub_loader_set (grub_secureboot_chainloader_boot,
- grub_secureboot_chainloader_unload, 0);
+ grub_loader_set_ex (grub_secureboot_chainloader_boot,
+ grub_secureboot_chainloader_unload, sb_context, 0);
return 0;
}
else if (rc == 0)
@@ -1123,7 +1132,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
efi_call_2 (b->free_pages, address, pages);
grub_free (file_path);
- grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
+ grub_loader_set_ex (grub_chainloader_boot, grub_chainloader_unload, image_handle, 0);
return 0;
}
@@ -1145,10 +1154,7 @@ fail:
grub_free (cmdline);
if (image_handle != 0)
- {
- efi_call_1 (b->unload_image, image_handle);
- image_handle = 0;
- }
+ efi_call_1 (b->unload_image, image_handle);
grub_dl_unref (my_mod);

View File

@ -0,0 +1,44 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Mon, 2 May 2022 14:39:31 +0200
Subject: [PATCH] loader/i386/efi/linux: Avoid a use-after-free in the linuxefi
loader
In some error paths in grub_cmd_linux, the pointer to lh may be
dereferenced after the buffer it points to has been freed. There aren't
any security implications from this because nothing else uses the
allocator after the buffer is freed and before the pointer is
dereferenced, but fix it anyway.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit 8224f5a71af94bec8697de17e7e579792db9f9e2)
(cherry picked from commit 4744b62e20d07674017213ac54d7442d679f9d1a)
(cherry picked from commit 329633cb060957c3d2aca677ac733f07b213a63f)
(cherry picked from commit 47b839b0a801ee4852447a85fb5de91dc7d2c856)
---
grub-core/loader/i386/efi/linux.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c
index a043df891f..c9a2b47370 100644
--- a/grub-core/loader/i386/efi/linux.c
+++ b/grub-core/loader/i386/efi/linux.c
@@ -482,9 +482,6 @@ fail:
if (file)
grub_file_close (file);
- if (kernel)
- grub_free (kernel);
-
if (grub_errno != GRUB_ERR_NONE)
{
grub_dl_unref (my_mod);
@@ -500,6 +497,8 @@ fail:
kernel_free (params, sizeof(*params));
}
+ grub_free (kernel);
+
return grub_errno;
}

View File

@ -0,0 +1,300 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Mon, 2 May 2022 17:04:23 +0200
Subject: [PATCH] loader/i386/efi/linux: Use grub_loader_set_ex
This ports the linuxefi loader to use grub_loader_set_ex in order to fix
a use-after-fre bug that occurs when grub_cmd_linux is executed more than
once before a boot attempt is performed.
This is more complicated than for the chainloader command, as the initrd
command needs access to the loader state. To solve this, the linuxefi
module registers a dummy initrd command at startup that returns an error.
The linuxefi command then registers a proper initrd command with a higher
priority that is passed the loader state.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit 7cf736436b4c934df5ddfa6f44b46a7e07d99fdc)
[rharwood/pjones: set kernel_size in context]
(cherry picked from commit 9c056391f7a36ea480de9a759c12e55a90f2040a)
[rharwood: verifying twice]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit df804892f1a754d88a9779320f9429bf40d2a1b3)
(cherry picked from commit d1b506f6c910b96ad47a20247b438c6402a74948)
---
grub-core/loader/i386/efi/linux.c | 146 +++++++++++++++++++++++---------------
1 file changed, 87 insertions(+), 59 deletions(-)
diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c
index c9a2b47370..77a0734786 100644
--- a/grub-core/loader/i386/efi/linux.c
+++ b/grub-core/loader/i386/efi/linux.c
@@ -34,13 +34,19 @@
GRUB_MOD_LICENSE ("GPLv3+");
static grub_dl_t my_mod;
-static int loaded;
-static void *kernel_mem;
-static grub_uint64_t kernel_size;
-static void *initrd_mem;
-static grub_uint32_t handover_offset;
-struct linux_kernel_params *params;
-static char *linux_cmdline;
+
+static grub_command_t cmd_linux, cmd_initrd;
+static grub_command_t cmd_linuxefi, cmd_initrdefi;
+
+struct grub_linuxefi_context {
+ void *kernel_mem;
+ grub_uint64_t kernel_size;
+ grub_uint32_t handover_offset;
+ struct linux_kernel_params *params;
+ char *cmdline;
+
+ void *initrd_mem;
+};
#define MIN(a, b) \
({ typeof (a) _a = (a); \
@@ -123,25 +129,32 @@ kernel_alloc(grub_efi_uintn_t size, const char * const errmsg)
}
static grub_err_t
-grub_linuxefi_boot (void)
+grub_linuxefi_boot (void *data)
{
+ struct grub_linuxefi_context *context = (struct grub_linuxefi_context *) data;
+
asm volatile ("cli");
- return grub_efi_linux_boot ((char *)kernel_mem,
- handover_offset,
- params);
+ return grub_efi_linux_boot ((char *)context->kernel_mem,
+ context->handover_offset,
+ context->params);
}
static grub_err_t
-grub_linuxefi_unload (void)
+grub_linuxefi_unload (void *data)
{
+ struct grub_linuxefi_context *context = (struct grub_linuxefi_context *) data;
+ struct linux_kernel_params *params = context->params;
+
grub_dl_unref (my_mod);
- loaded = 0;
- kernel_free(initrd_mem, params->ramdisk_size);
- kernel_free(linux_cmdline, params->cmdline_size + 1);
- kernel_free(kernel_mem, kernel_size);
- kernel_free(params, sizeof(*params));
+ kernel_free (context->initrd_mem, params->ramdisk_size);
+ kernel_free (context->cmdline, params->cmdline_size + 1);
+ kernel_free (context->kernel_mem, context->kernel_size);
+ kernel_free (params, sizeof(*params));
+ cmd_initrd->data = 0;
+ cmd_initrdefi->data = 0;
+ grub_free (context);
return GRUB_ERR_NONE;
}
@@ -188,13 +201,14 @@ read(grub_file_t file, grub_uint8_t *bufp, grub_size_t len)
#define HIGH_U32(val) ((grub_uint32_t)(((grub_addr_t)(val) >> 32) & 0xffffffffull))
static grub_err_t
-grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
- int argc, char *argv[])
+grub_cmd_initrd (grub_command_t cmd, int argc, char *argv[])
{
grub_file_t *files = 0;
int i, nfiles = 0;
grub_size_t size = 0;
grub_uint8_t *ptr;
+ struct grub_linuxefi_context *context = (struct grub_linuxefi_context *) cmd->data;
+ struct linux_kernel_params *params;
if (argc == 0)
{
@@ -202,12 +216,14 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}
- if (!loaded)
+ if (!context)
{
grub_error (GRUB_ERR_BAD_ARGUMENT, N_("you need to load the kernel first"));
goto fail;
}
+ params = context->params;
+
files = grub_calloc (argc, sizeof (files[0]));
if (!files)
goto fail;
@@ -226,19 +242,19 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
}
}
- initrd_mem = kernel_alloc(size, N_("can't allocate initrd"));
- if (initrd_mem == NULL)
+ context->initrd_mem = kernel_alloc(size, N_("can't allocate initrd"));
+ if (context->initrd_mem == NULL)
goto fail;
- grub_dprintf ("linux", "initrd_mem = %p\n", initrd_mem);
+ grub_dprintf ("linux", "initrd_mem = %p\n", context->initrd_mem);
params->ramdisk_size = LOW_U32(size);
- params->ramdisk_image = LOW_U32(initrd_mem);
+ params->ramdisk_image = LOW_U32(context->initrd_mem);
#if defined(__x86_64__)
params->ext_ramdisk_size = HIGH_U32(size);
- params->ext_ramdisk_image = HIGH_U32(initrd_mem);
+ params->ext_ramdisk_image = HIGH_U32(context->initrd_mem);
#endif
- ptr = initrd_mem;
+ ptr = context->initrd_mem;
for (i = 0; i < nfiles; i++)
{
@@ -264,8 +280,8 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
grub_file_close (files[i]);
grub_free (files);
- if (initrd_mem && grub_errno)
- grub_efi_free_pages ((grub_efi_physical_address_t)(grub_addr_t)initrd_mem,
+ if (context->initrd_mem && grub_errno)
+ grub_efi_free_pages ((grub_efi_physical_address_t)(grub_addr_t)context->initrd_mem,
BYTES_TO_PAGES(size));
return grub_errno;
@@ -281,6 +297,12 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
void *kernel = NULL;
int setup_header_end_offset;
int rc;
+ void *kernel_mem = 0;
+ grub_uint64_t kernel_size = 0;
+ grub_uint32_t handover_offset;
+ struct linux_kernel_params *params = 0;
+ char *cmdline = 0;
+ struct grub_linuxefi_context *context = 0;
grub_dl_ref (my_mod);
@@ -407,27 +429,27 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
grub_dprintf ("linux", "new lh is at %p\n", lh);
grub_dprintf ("linux", "setting up cmdline\n");
- linux_cmdline = kernel_alloc (lh->cmdline_size + 1, N_("can't allocate cmdline"));
- if (!linux_cmdline)
+ cmdline = kernel_alloc (lh->cmdline_size + 1, N_("can't allocate cmdline"));
+ if (!cmdline)
goto fail;
- grub_dprintf ("linux", "linux_cmdline = %p\n", linux_cmdline);
+ grub_dprintf ("linux", "cmdline = %p\n", cmdline);
- grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
+ grub_memcpy (cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
grub_create_loader_cmdline (argc, argv,
- linux_cmdline + sizeof (LINUX_IMAGE) - 1,
+ cmdline + sizeof (LINUX_IMAGE) - 1,
lh->cmdline_size - (sizeof (LINUX_IMAGE) - 1),
GRUB_VERIFY_KERNEL_CMDLINE);
- grub_dprintf ("linux", "cmdline:%s\n", linux_cmdline);
+ grub_dprintf ("linux", "cmdline:%s\n", cmdline);
grub_dprintf ("linux", "setting lh->cmd_line_ptr to 0x%08x\n",
- LOW_U32(linux_cmdline));
- lh->cmd_line_ptr = LOW_U32(linux_cmdline);
+ LOW_U32(cmdline));
+ lh->cmd_line_ptr = LOW_U32(cmdline);
#if defined(__x86_64__)
- if ((grub_efi_uintn_t)linux_cmdline > 0xffffffffull)
+ if ((grub_efi_uintn_t)cmdline > 0xffffffffull)
{
grub_dprintf ("linux", "setting params->ext_cmd_line_ptr to 0x%08x\n",
- HIGH_U32(linux_cmdline));
- params->ext_cmd_line_ptr = HIGH_U32(linux_cmdline);
+ HIGH_U32(cmdline));
+ params->ext_cmd_line_ptr = HIGH_U32(cmdline);
}
#endif
@@ -452,16 +474,13 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
}
max_addresses[1].addr = GRUB_EFI_MAX_ALLOCATION_ADDRESS;
max_addresses[2].addr = GRUB_EFI_MAX_ALLOCATION_ADDRESS;
- kernel_mem = kernel_alloc (lh->init_size, N_("can't allocate kernel"));
+ kernel_size = lh->init_size;
+ kernel_mem = kernel_alloc (kernel_size, N_("can't allocate kernel"));
restore_addresses();
if (!kernel_mem)
goto fail;
grub_dprintf("linux", "kernel_mem = %p\n", kernel_mem);
- grub_loader_set (grub_linuxefi_boot, grub_linuxefi_unload, 0);
-
- loaded = 1;
-
grub_dprintf ("linux", "setting lh->code32_start to 0x%08x\n",
LOW_U32(kernel_mem));
lh->code32_start = LOW_U32(kernel_mem);
@@ -478,33 +497,42 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
"setting lh->ext_loader_{type,ver} = {0x%02x,0x%02x}\n",
params->ext_loader_type, params->ext_loader_ver);
+ context = grub_zalloc (sizeof (*context));
+ if (!context)
+ goto fail;
+ context->kernel_mem = kernel_mem;
+ context->kernel_size = kernel_size;
+ context->handover_offset = handover_offset;
+ context->params = params;
+ context->cmdline = cmdline;
+
+ grub_loader_set_ex (grub_linuxefi_boot, grub_linuxefi_unload, context, 0);
+
+ cmd_initrd->data = context;
+ cmd_initrdefi->data = context;
+
+ grub_file_close (file);
+ grub_free (kernel);
+ return 0;
+
fail:
if (file)
grub_file_close (file);
- if (grub_errno != GRUB_ERR_NONE)
- {
- grub_dl_unref (my_mod);
- loaded = 0;
- }
+ grub_dl_unref (my_mod);
- if (!loaded)
- {
- if (lh)
- kernel_free (linux_cmdline, lh->cmdline_size + 1);
+ if (lh)
+ kernel_free (cmdline, lh->cmdline_size + 1);
- kernel_free (kernel_mem, kernel_size);
- kernel_free (params, sizeof(*params));
- }
+ kernel_free (kernel_mem, kernel_size);
+ kernel_free (params, sizeof(*params));
+ grub_free (context);
grub_free (kernel);
return grub_errno;
}
-static grub_command_t cmd_linux, cmd_initrd;
-static grub_command_t cmd_linuxefi, cmd_initrdefi;
-
GRUB_MOD_INIT(linux)
{
cmd_linux =

View File

@ -0,0 +1,78 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Tue, 3 May 2022 09:47:35 +0200
Subject: [PATCH] loader/i386/efi/linux: Fix a memory leak in the initrd
command
Subsequent invocations of the initrd command result in the previous
initrd being leaked, so fix that.
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
(cherry picked from commit d98af31ce1e31bb22163960d53f5eb28c66582a0)
(cherry picked from commit 62234d6a00e6d1dd8e017ff161d359feb5234082)
(cherry picked from commit bda5a10716dc9676400dce1374232452f46d0bc4)
(cherry picked from commit b862299a8502282a09af8e6c6189edd5b0a368b0)
---
grub-core/loader/i386/efi/linux.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c
index 77a0734786..8337191921 100644
--- a/grub-core/loader/i386/efi/linux.c
+++ b/grub-core/loader/i386/efi/linux.c
@@ -209,6 +209,7 @@ grub_cmd_initrd (grub_command_t cmd, int argc, char *argv[])
grub_uint8_t *ptr;
struct grub_linuxefi_context *context = (struct grub_linuxefi_context *) cmd->data;
struct linux_kernel_params *params;
+ void *initrd_mem = 0;
if (argc == 0)
{
@@ -242,19 +243,19 @@ grub_cmd_initrd (grub_command_t cmd, int argc, char *argv[])
}
}
- context->initrd_mem = kernel_alloc(size, N_("can't allocate initrd"));
- if (context->initrd_mem == NULL)
+ initrd_mem = kernel_alloc(size, N_("can't allocate initrd"));
+ if (initrd_mem == NULL)
goto fail;
- grub_dprintf ("linux", "initrd_mem = %p\n", context->initrd_mem);
+ grub_dprintf ("linux", "initrd_mem = %p\n", initrd_mem);
params->ramdisk_size = LOW_U32(size);
- params->ramdisk_image = LOW_U32(context->initrd_mem);
+ params->ramdisk_image = LOW_U32(initrd_mem);
#if defined(__x86_64__)
params->ext_ramdisk_size = HIGH_U32(size);
- params->ext_ramdisk_image = HIGH_U32(context->initrd_mem);
+ params->ext_ramdisk_image = HIGH_U32(initrd_mem);
#endif
- ptr = context->initrd_mem;
+ ptr = initrd_mem;
for (i = 0; i < nfiles; i++)
{
@@ -273,6 +274,9 @@ grub_cmd_initrd (grub_command_t cmd, int argc, char *argv[])
ptr += ALIGN_UP_OVERHEAD (cursize, 4);
}
+ kernel_free(context->initrd_mem, params->ramdisk_size);
+
+ context->initrd_mem = initrd_mem;
params->ramdisk_size = size;
fail:
@@ -280,9 +284,8 @@ grub_cmd_initrd (grub_command_t cmd, int argc, char *argv[])
grub_file_close (files[i]);
grub_free (files);
- if (context->initrd_mem && grub_errno)
- grub_efi_free_pages ((grub_efi_physical_address_t)(grub_addr_t)context->initrd_mem,
- BYTES_TO_PAGES(size));
+ if (initrd_mem && grub_errno)
+ kernel_free (initrd_mem, size);
return grub_errno;
}

View File

@ -0,0 +1,42 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Fri, 25 Jun 2021 02:19:05 +1000
Subject: [PATCH] kern/file: Do not leak device_name on error in
grub_file_open()
If we have an error in grub_file_open() before we free device_name, we
will leak it.
Free device_name in the error path and null out the pointer in the good
path once we free it there.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 1499a5068839fa37cb77ecef4b5bdacbd1ed12ea)
(cherry picked from commit 2ec50b289d8b24922433439533113087f111f110)
(cherry picked from commit 17c36ae88d7d6040cabc01cd4a21e71ff4731668)
(cherry picked from commit 723e7dbedb7669343e564d453d21b8ed2ab81216)
---
grub-core/kern/file.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c
index 2efc31da94..f062fc21e7 100644
--- a/grub-core/kern/file.c
+++ b/grub-core/kern/file.c
@@ -81,6 +81,7 @@ grub_file_open (const char *name, enum grub_file_type type)
device = grub_device_open (device_name);
grub_free (device_name);
+ device_name = NULL;
if (! device)
goto fail;
@@ -135,6 +136,7 @@ grub_file_open (const char *name, enum grub_file_type type)
return file;
fail:
+ grub_free (device_name);
if (device)
grub_device_close (device);

View File

@ -0,0 +1,201 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 14:02:55 +1000
Subject: [PATCH] video/readers/png: Abort sooner if a read operation fails
Fuzzing revealed some inputs that were taking a long time, potentially
forever, because they did not bail quickly upon encountering an I/O error.
Try to catch I/O errors sooner and bail out.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 882be97d1df6449b9fd4d593f0cb70005fde3494)
(cherry picked from commit 3f6fc3ebfd58fcdb3fe6c2f7a5a4fa05772ae786)
(cherry picked from commit aac5b8257d4078c3f764216aeae3367bdc19043f)
(cherry picked from commit e9e58c9711de334fcf48a651ee20c21f2855a4bd)
---
grub-core/video/readers/png.c | 55 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index 0157ff7420..e2a6b1cf3c 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -142,6 +142,7 @@ static grub_uint8_t
grub_png_get_byte (struct grub_png_data *data)
{
grub_uint8_t r;
+ grub_ssize_t bytes_read = 0;
if ((data->inside_idat) && (data->idat_remain == 0))
{
@@ -175,7 +176,14 @@ grub_png_get_byte (struct grub_png_data *data)
}
r = 0;
- grub_file_read (data->file, &r, 1);
+ bytes_read = grub_file_read (data->file, &r, 1);
+
+ if (bytes_read != 1)
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "png: unexpected end of data");
+ return 0;
+ }
if (data->inside_idat)
data->idat_remain--;
@@ -231,15 +239,16 @@ grub_png_decode_image_palette (struct grub_png_data *data,
if (len == 0)
return GRUB_ERR_NONE;
- for (i = 0; 3 * i < len && i < 256; i++)
+ grub_errno = GRUB_ERR_NONE;
+ for (i = 0; 3 * i < len && i < 256 && grub_errno == GRUB_ERR_NONE; i++)
for (j = 0; j < 3; j++)
data->palette[i][j] = grub_png_get_byte (data);
- for (i *= 3; i < len; i++)
+ for (i *= 3; i < len && grub_errno == GRUB_ERR_NONE; i++)
grub_png_get_byte (data);
grub_png_get_dword (data);
- return GRUB_ERR_NONE;
+ return grub_errno;
}
static grub_err_t
@@ -256,9 +265,13 @@ grub_png_decode_image_header (struct grub_png_data *data)
return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid image size");
color_bits = grub_png_get_byte (data);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
data->is_16bit = (color_bits == 16);
color_type = grub_png_get_byte (data);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
/* According to PNG spec, no other types are valid. */
if ((color_type & ~(PNG_COLOR_MASK_ALPHA | PNG_COLOR_MASK_COLOR))
@@ -340,14 +353,20 @@ grub_png_decode_image_header (struct grub_png_data *data)
if (grub_png_get_byte (data) != PNG_COMPRESSION_BASE)
return grub_error (GRUB_ERR_BAD_FILE_TYPE,
"png: compression method not supported");
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
if (grub_png_get_byte (data) != PNG_FILTER_TYPE_BASE)
return grub_error (GRUB_ERR_BAD_FILE_TYPE,
"png: filter method not supported");
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
if (grub_png_get_byte (data) != PNG_INTERLACE_NONE)
return grub_error (GRUB_ERR_BAD_FILE_TYPE,
"png: interlace method not supported");
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
/* Skip crc checksum. */
grub_png_get_dword (data);
@@ -449,7 +468,7 @@ grub_png_get_huff_code (struct grub_png_data *data, struct huff_table *ht)
int code, i;
code = 0;
- for (i = 0; i < ht->max_length; i++)
+ for (i = 0; i < ht->max_length && grub_errno == GRUB_ERR_NONE; i++)
{
code = (code << 1) + grub_png_get_bits (data, 1);
if (code < ht->maxval[i])
@@ -504,8 +523,14 @@ grub_png_init_dynamic_block (struct grub_png_data *data)
grub_uint8_t lens[DEFLATE_HCLEN_MAX];
nl = DEFLATE_HLIT_BASE + grub_png_get_bits (data, 5);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
nd = DEFLATE_HDIST_BASE + grub_png_get_bits (data, 5);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
nb = DEFLATE_HCLEN_BASE + grub_png_get_bits (data, 4);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
if ((nl > DEFLATE_HLIT_MAX) || (nd > DEFLATE_HDIST_MAX) ||
(nb > DEFLATE_HCLEN_MAX))
@@ -533,7 +558,7 @@ grub_png_init_dynamic_block (struct grub_png_data *data)
data->dist_offset);
prev = 0;
- for (i = 0; i < nl + nd; i++)
+ for (i = 0; i < nl + nd && grub_errno == GRUB_ERR_NONE; i++)
{
int n, code;
struct huff_table *ht;
@@ -721,17 +746,21 @@ grub_png_read_dynamic_block (struct grub_png_data *data)
len = cplens[n];
if (cplext[n])
len += grub_png_get_bits (data, cplext[n]);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
n = grub_png_get_huff_code (data, &data->dist_table);
dist = cpdist[n];
if (cpdext[n])
dist += grub_png_get_bits (data, cpdext[n]);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
pos = data->wp - dist;
if (pos < 0)
pos += WSIZE;
- while (len > 0)
+ while (len > 0 && grub_errno == GRUB_ERR_NONE)
{
data->slide[data->wp] = data->slide[pos];
grub_png_output_byte (data, data->slide[data->wp]);
@@ -759,7 +788,11 @@ grub_png_decode_image_data (struct grub_png_data *data)
int final;
cmf = grub_png_get_byte (data);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
flg = grub_png_get_byte (data);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
if ((cmf & 0xF) != Z_DEFLATED)
return grub_error (GRUB_ERR_BAD_FILE_TYPE,
@@ -774,7 +807,11 @@ grub_png_decode_image_data (struct grub_png_data *data)
int block_type;
final = grub_png_get_bits (data, 1);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
block_type = grub_png_get_bits (data, 2);
+ if (grub_errno != GRUB_ERR_NONE)
+ return grub_errno;
switch (block_type)
{
@@ -790,7 +827,7 @@ grub_png_decode_image_data (struct grub_png_data *data)
grub_png_get_byte (data);
grub_png_get_byte (data);
- for (i = 0; i < len; i++)
+ for (i = 0; i < len && grub_errno == GRUB_ERR_NONE; i++)
grub_png_output_byte (data, grub_png_get_byte (data));
break;
@@ -1045,6 +1082,8 @@ grub_png_decode_png (struct grub_png_data *data)
len = grub_png_get_dword (data);
type = grub_png_get_dword (data);
+ if (grub_errno != GRUB_ERR_NONE)
+ break;
data->next_offset = data->file->offset + len + 4;
switch (type)

View File

@ -0,0 +1,31 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 14:13:40 +1000
Subject: [PATCH] video/readers/png: Refuse to handle multiple image headers
This causes the bitmap to be leaked. Do not permit multiple image headers.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 8ce433557adeadbc46429aabb9f850b02ad2bdfb)
(cherry picked from commit 6e10bba6a4cbfd6c7bf116f41fd4e037465e19d8)
(cherry picked from commit 812272d919ecfd368c008f15b677d369616ada54)
(cherry picked from commit c04569b35600aa29d5b4cd8990a8ee1dd1162c72)
---
grub-core/video/readers/png.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index e2a6b1cf3c..8955b8ecfd 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -258,6 +258,9 @@ grub_png_decode_image_header (struct grub_png_data *data)
int color_bits;
enum grub_video_blit_format blt;
+ if (data->image_width || data->image_height)
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: two image headers found");
+
data->image_width = grub_png_get_dword (data);
data->image_height = grub_png_get_dword (data);

View File

@ -0,0 +1,173 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 18:51:35 +1000
Subject: [PATCH] video/readers/png: Drop greyscale support to fix heap
out-of-bounds write
A 16-bit greyscale PNG without alpha is processed in the following loop:
for (i = 0; i < (data->image_width * data->image_height);
i++, d1 += 4, d2 += 2)
{
d1[R3] = d2[1];
d1[G3] = d2[1];
d1[B3] = d2[1];
}
The increment of d1 is wrong. d1 is incremented by 4 bytes per iteration,
but there are only 3 bytes allocated for storage. This means that image
data will overwrite somewhat-attacker-controlled parts of memory - 3 bytes
out of every 4 following the end of the image.
This has existed since greyscale support was added in 2013 in commit
3ccf16dff98f (grub-core/video/readers/png.c: Support grayscale).
Saving starfield.png as a 16-bit greyscale image without alpha in the gimp
and attempting to load it causes grub-emu to crash - I don't think this code
has ever worked.
Delete all PNG greyscale support.
Fixes: CVE-2021-3695
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 0e1d163382669bd734439d8864ee969616d971d9)
[rharwood: context conflict]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit 4c631c8119206b3178912df2905434d967661c3d)
(cherry picked from commit 6d5d5f51266b8113c6ba560835500e3c135f3722)
(cherry picked from commit b20fc5589561a8c57a2071b2ae93fcdcf51a10d4)
---
grub-core/video/readers/png.c | 85 +++----------------------------------------
1 file changed, 6 insertions(+), 79 deletions(-)
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index 8955b8ecfd..a3161e25b6 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -100,7 +100,7 @@ struct grub_png_data
unsigned image_width, image_height;
int bpp, is_16bit;
- int raw_bytes, is_gray, is_alpha, is_palette;
+ int raw_bytes, is_alpha, is_palette;
int row_bytes, color_bits;
grub_uint8_t *image_data;
@@ -296,13 +296,13 @@ grub_png_decode_image_header (struct grub_png_data *data)
data->bpp = 3;
else
{
- data->is_gray = 1;
- data->bpp = 1;
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "png: color type not supported");
}
if ((color_bits != 8) && (color_bits != 16)
&& (color_bits != 4
- || !(data->is_gray || data->is_palette)))
+ || !data->is_palette))
return grub_error (GRUB_ERR_BAD_FILE_TYPE,
"png: bit depth must be 8 or 16");
@@ -331,7 +331,7 @@ grub_png_decode_image_header (struct grub_png_data *data)
}
#ifndef GRUB_CPU_WORDS_BIGENDIAN
- if (data->is_16bit || data->is_gray || data->is_palette)
+ if (data->is_16bit || data->is_palette)
#endif
{
data->image_data = grub_calloc (data->image_height, data->row_bytes);
@@ -899,27 +899,8 @@ grub_png_convert_image (struct grub_png_data *data)
int shift;
int mask = (1 << data->color_bits) - 1;
unsigned j;
- if (data->is_gray)
- {
- /* Generic formula is
- (0xff * i) / ((1U << data->color_bits) - 1)
- but for allowed bit depth of 1, 2 and for it's
- equivalent to
- (0xff / ((1U << data->color_bits) - 1)) * i
- Precompute the multipliers to avoid division.
- */
- const grub_uint8_t multipliers[5] = { 0xff, 0xff, 0x55, 0x24, 0x11 };
- for (i = 0; i < (1U << data->color_bits); i++)
- {
- grub_uint8_t col = multipliers[data->color_bits] * i;
- palette[i][0] = col;
- palette[i][1] = col;
- palette[i][2] = col;
- }
- }
- else
- grub_memcpy (palette, data->palette, 3 << data->color_bits);
+ grub_memcpy (palette, data->palette, 3 << data->color_bits);
d1c = d1;
d2c = d2;
for (j = 0; j < data->image_height; j++, d1c += data->image_width * 3,
@@ -956,60 +937,6 @@ grub_png_convert_image (struct grub_png_data *data)
}
return;
}
-
- if (data->is_gray)
- {
- switch (data->bpp)
- {
- case 4:
- /* 16-bit gray with alpha. */
- for (i = 0; i < (data->image_width * data->image_height);
- i++, d1 += 4, d2 += 4)
- {
- d1[R4] = d2[3];
- d1[G4] = d2[3];
- d1[B4] = d2[3];
- d1[A4] = d2[1];
- }
- break;
- case 2:
- if (data->is_16bit)
- /* 16-bit gray without alpha. */
- {
- for (i = 0; i < (data->image_width * data->image_height);
- i++, d1 += 4, d2 += 2)
- {
- d1[R3] = d2[1];
- d1[G3] = d2[1];
- d1[B3] = d2[1];
- }
- }
- else
- /* 8-bit gray with alpha. */
- {
- for (i = 0; i < (data->image_width * data->image_height);
- i++, d1 += 4, d2 += 2)
- {
- d1[R4] = d2[1];
- d1[G4] = d2[1];
- d1[B4] = d2[1];
- d1[A4] = d2[0];
- }
- }
- break;
- /* 8-bit gray without alpha. */
- case 1:
- for (i = 0; i < (data->image_width * data->image_height);
- i++, d1 += 3, d2++)
- {
- d1[R3] = d2[0];
- d1[G3] = d2[0];
- d1[B3] = d2[0];
- }
- break;
- }
- return;
- }
{
/* Only copy the upper 8 bit. */

View File

@ -0,0 +1,43 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 23:25:07 +1000
Subject: [PATCH] video/readers/png: Avoid heap OOB R/W inserting huff table
items
In fuzzing we observed crashes where a code would attempt to be inserted
into a huffman table before the start, leading to a set of heap OOB reads
and writes as table entries with negative indices were shifted around and
the new code written in.
Catch the case where we would underflow the array and bail.
Fixes: CVE-2021-3696
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 1ae9a91d42cb40da8a6f11fac65541858e340afa)
(cherry picked from commit 132ccc681cf642ad748580f26b54c9259a7f43fd)
(cherry picked from commit 3a70e1f6e69af6e0d3c3cf526faa44dc0c80ac19)
(cherry picked from commit 809d25ffa6b89d390a66d2f3cf3090196f07e2aa)
---
grub-core/video/readers/png.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index a3161e25b6..d7ed5aa6cf 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -438,6 +438,13 @@ grub_png_insert_huff_item (struct huff_table *ht, int code, int len)
for (i = len; i < ht->max_length; i++)
n += ht->maxval[i];
+ if (n > ht->num_values)
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "png: out of range inserting huffman table item");
+ return;
+ }
+
for (i = 0; i < n; i++)
ht->values[ht->num_values - i] = ht->values[ht->num_values - i - 1];

View File

@ -0,0 +1,43 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 19:19:11 +1000
Subject: [PATCH] video/readers/png: Sanity check some huffman codes
ASAN picked up two OOB global reads: we weren't checking if some code
values fit within the cplens or cpdext arrays. Check and throw an error
if not.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit c3a8ab0cbd24153ec7b1f84a96ddfdd72ef8d117)
(cherry picked from commit 5d09addf58086aa11d5f9a91af5632ff87c2d2ee)
(cherry picked from commit ff12584f9376a472f37d4ec14213fd29bf3b233a)
(cherry picked from commit ac8b5464a076d2e38ecf7f761be9cd1f5bbeb784)
---
grub-core/video/readers/png.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index d7ed5aa6cf..7f2ba7849b 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -753,6 +753,9 @@ grub_png_read_dynamic_block (struct grub_png_data *data)
int len, dist, pos;
n -= 257;
+ if (((unsigned int) n) >= ARRAY_SIZE (cplens))
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "png: invalid huff code");
len = cplens[n];
if (cplext[n])
len += grub_png_get_bits (data, cplext[n]);
@@ -760,6 +763,9 @@ grub_png_read_dynamic_block (struct grub_png_data *data)
return grub_errno;
n = grub_png_get_huff_code (data, &data->dist_table);
+ if (((unsigned int) n) >= ARRAY_SIZE (cpdist))
+ return grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "png: invalid huff code");
dist = cpdist[n];
if (cpdext[n])
dist += grub_png_get_bits (data, cpdext[n]);

View File

@ -0,0 +1,258 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Mon, 28 Jun 2021 14:16:14 +1000
Subject: [PATCH] video/readers/jpeg: Abort sooner if a read operation fails
Fuzzing revealed some inputs that were taking a long time, potentially
forever, because they did not bail quickly upon encountering an I/O error.
Try to catch I/O errors sooner and bail out.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit ab2e5d2e4bff488bbb557ed435a61ae102ef9f0c)
(cherry picked from commit 1ff8df0d2dea8ec7c8575241d5e7d6622c204ec3)
(cherry picked from commit b07767383b74a0ce7135c09ba8701510d4ad32f0)
(cherry picked from commit 5f097165152d61d4aea02f26dc789d840147d50e)
---
grub-core/video/readers/jpeg.c | 86 ++++++++++++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 16 deletions(-)
diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c
index e31602f766..10225abd53 100644
--- a/grub-core/video/readers/jpeg.c
+++ b/grub-core/video/readers/jpeg.c
@@ -109,9 +109,17 @@ static grub_uint8_t
grub_jpeg_get_byte (struct grub_jpeg_data *data)
{
grub_uint8_t r;
+ grub_ssize_t bytes_read;
r = 0;
- grub_file_read (data->file, &r, 1);
+ bytes_read = grub_file_read (data->file, &r, 1);
+
+ if (bytes_read != 1)
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "jpeg: unexpected end of data");
+ return 0;
+ }
return r;
}
@@ -120,9 +128,17 @@ static grub_uint16_t
grub_jpeg_get_word (struct grub_jpeg_data *data)
{
grub_uint16_t r;
+ grub_ssize_t bytes_read;
r = 0;
- grub_file_read (data->file, &r, sizeof (grub_uint16_t));
+ bytes_read = grub_file_read (data->file, &r, sizeof (grub_uint16_t));
+
+ if (bytes_read != sizeof (grub_uint16_t))
+ {
+ grub_error (GRUB_ERR_BAD_FILE_TYPE,
+ "jpeg: unexpected end of data");
+ return 0;
+ }