Re: [PATCH] font: Try opening fonts from the bundled memdisk

[Steve McIntyre <steve@einval.com>]

We're running with this patch in Debian builds right now and it's
working well...

Reviewed-by: Steve McIntyre <93sam@debian.org>
Tested-by: Steve McIntyre <93sam@debian.org>

(whichever you prefer!)

On Wed, Apr 26, 2023 at 12:06:52PM +0200, Chris Coulson wrote:
>Grub since 93a786a00163e50c29f0394df198518617e1c9a5 has enforced
>verification of font files in secure boot mode. In order to continue to
>be able to load some default fonts, vendors may bundle them with their
>signed EFI image by adding them to the built-in memdisk.
>
>This change makes the font loader try loading fonts from the memdisk
>before the prefix path when attempting to load a font file by specifying
>its filename, which avoids having to make changes to grub configurations
>in order to accommodate memdisk bundled fonts. It expects the directory
>structure to be the same as fonts stored in the prefix path
>(ie, /fonts/<name>.pf2).
>
>Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
>---
> grub-core/font/font.c | 48 ++++++++++++++++++++++++++++---------------
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
>diff --git a/grub-core/font/font.c b/grub-core/font/font.c
>index 24adcb35a..7c83467a3 100644
>--- a/grub-core/font/font.c
>+++ b/grub-core/font/font.c
>@@ -415,6 +415,27 @@ read_section_as_short (struct font_file_section *section,
>   return 0;
> }
> 
>+static grub_file_t
>+try_open_from_prefix (const char *prefix, const char *filename)
>+{
>+  grub_file_t file;
>+  char *fullname, *ptr;
>+
>+  fullname = grub_malloc (grub_strlen (prefix) + grub_strlen (filename) + 1
>+			  + sizeof ("/fonts/") + sizeof (".pf2"));
>+  if (!fullname)
>+    return 0;
>+  ptr = grub_stpcpy (fullname, prefix);
>+  ptr = grub_stpcpy (ptr, "/fonts/");
>+  ptr = grub_stpcpy (ptr, filename);
>+  ptr = grub_stpcpy (ptr, ".pf2");
>+  *ptr = 0;
>+
>+  file = grub_buffile_open (fullname, GRUB_FILE_TYPE_FONT, 1024);
>+  grub_free (fullname);
>+  return file;
>+}
>+
> /* Load a font and add it to the beginning of the global font list.
>    Returns 0 upon success, nonzero upon failure.  */
> grub_font_t
>@@ -433,25 +454,18 @@ grub_font_load (const char *filename)
>     file = grub_buffile_open (filename, GRUB_FILE_TYPE_FONT, 1024);
>   else
>     {
>-      const char *prefix = grub_env_get ("prefix");
>-      char *fullname, *ptr;
>-      if (!prefix)
>+      file = try_open_from_prefix ("(memdisk)", filename);
>+      if (!file)
> 	{
>-	  grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"),
>-		      "prefix");
>-	  goto fail;
>+	  const char *prefix = grub_env_get ("prefix");
>+	  if (!prefix)
>+	    {
>+	      grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"),
>+			  "prefix");
>+	      goto fail;
>+	    }
>+	  file = try_open_from_prefix (prefix, filename);
> 	}
>-      fullname = grub_malloc (grub_strlen (prefix) + grub_strlen (filename) + 1
>-			      + sizeof ("/fonts/") + sizeof (".pf2"));
>-      if (!fullname)
>-	goto fail;
>-      ptr = grub_stpcpy (fullname, prefix);
>-      ptr = grub_stpcpy (ptr, "/fonts/");
>-      ptr = grub_stpcpy (ptr, filename);
>-      ptr = grub_stpcpy (ptr, ".pf2");
>-      *ptr = 0;
>-      file = grub_buffile_open (fullname, GRUB_FILE_TYPE_FONT, 1024);
>-      grub_free (fullname);
>     }
>   if (!file)
>     goto fail;
>-- 
>2.39.2
>
>
>_______________________________________________
>Grub-devel mailing list
>Grub-devel@gnu.org
>https://lists.gnu.org/mailman/listinfo/grub-devel
-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
'There is some grim amusement in watching Pence try to run the typical
 "politician in the middle of a natural disaster" playbook, however
 incompetently, while Trump scribbles all over it in crayon and eats some
 of the pages.'   -- Russ Allbery