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

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

[Chris Coulson]

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

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

[Steve McIntyre]

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

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

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

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

> 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>

Thanks for posting this.

Reviewed-by: Robbie Harwood <rharwood@redhat.com>

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

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

[Daniel Kiper]

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>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel