public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
* [rpms/grub2] f45: term/serial: Reject caller-supplied port addresses under lockdown
@ 2026-09-24 15:47 Leo Sandoval
  0 siblings, 0 replies; only message in thread
From: Leo Sandoval @ 2026-09-24 15:47 UTC (permalink / raw)
  To: git-commits

            A new commit has been pushed.

            Repo   : rpms/grub2
            Branch : f45
            Commit : 65b159fd2228e987831cc400fa0dc89be296affb
            Author : Leo Sandoval <lsandova@redhat.com>
            Date   : 2026-09-23T14:32:17-06:00
            Stats  : +255/-2 in 3 file(s)
            URL    : https://src.fedoraproject.org/rpms/grub2/c/65b159fd2228e987831cc400fa0dc89be296affb?branch=f45

            Log:
            term/serial: Reject caller-supplied port addresses under lockdown

Signed-off-by: Leo Sandoval <lsandova@redhat.com>

---
diff --git a/0452-term-serial-Reject-caller-supplied-port-addresses-un.patch b/0452-term-serial-Reject-caller-supplied-port-addresses-un.patch
new file mode 100644
index 0000000..91a8d21
--- /dev/null
+++ b/0452-term-serial-Reject-caller-supplied-port-addresses-un.patch
@@ -0,0 +1,249 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Leo Sandoval <lsandova@redhat.com>
+Date: Wed, 23 Sep 2026 13:49:42 -0600
+Subject: [PATCH] term/serial: Reject caller-supplied port addresses under
+ lockdown
+
+The serial command is available in lockdown mode and the port option
+accepts a raw port address or a raw mmio base address. This can be
+misused to allow an attacker to write to local memory and do things
+including disabling / bypassing secure boot as long as the
+console is available or an unsigned GRUB configuration is under
+the attacker's control.
+
+Update the serial logic to only accept a raw address in lockdown when
+a port is already registered at it, which means the platform described
+that UART: grub_ns8250_init() for the legacy com<N> ports,
+grub_pciserial_init() for an enumerated PCI device, or an ACPI SPCR
+UART once "auto" has parsed the table. Names that are not addresses
+are untouched, so "auto", efi<N> from grub_efiserial_init() - the
+route on arm64, which has neither ns8250 nor SPCR support - and
+lookups by registered name all keep working.
+
+Behavior without lockdown (no secure boot) is unchanged.
+
+Assisted-by: Claude:Opus-5
+
+Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
+Suggested-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
+Reviewed-by: Leo Sandoval <lsandova@redhat.com>
+Part-of: <https://gitlab.freedesktop.org/gnu-grub/grub/-/merge_requests/266>
+---
+ grub-core/term/ns8250-spcr.c |  6 ++++--
+ grub-core/term/ns8250.c      | 27 ++++++++++++++++++++----
+ grub-core/term/serial.c      | 50 +++++++++++++++++++++++++++++++++++++++-----
+ include/grub/serial.h        |  8 +++++--
+ 4 files changed, 78 insertions(+), 13 deletions(-)
+
+diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c
+index 86f1aa078..b2f58965c 100644
+--- a/grub-core/term/ns8250-spcr.c
++++ b/grub-core/term/ns8250-spcr.c
+@@ -82,11 +82,13 @@ grub_ns8250_spcr_init (void)
+ 
+   switch (spcr->base_addr.space_id)
+     {
++      /* The address comes from the platform's own ACPI table, so it is trusted. */
+       case GRUB_ACPI_GENADDR_MEM_SPACE:
+         return grub_serial_ns8250_add_mmio (spcr->base_addr.addr,
+-                                            spcr->base_addr.access_size, &config);
++                                            spcr->base_addr.access_size, &config,
++                                            true);
+       case GRUB_ACPI_GENADDR_IO_SPACE:
+-        return grub_serial_ns8250_add_port (spcr->base_addr.addr, &config);
++        return grub_serial_ns8250_add_port (spcr->base_addr.addr, &config, true);
+       default:
+         return NULL;
+     };
+diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c
+index 23e8e0904..014bb0461 100644
+--- a/grub-core/term/ns8250.c
++++ b/grub-core/term/ns8250.c
+@@ -347,7 +347,8 @@ grub_ns8250_hw_get_port (const unsigned int unit)
+ }
+ 
+ struct grub_serial_port *
+-grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config)
++grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config,
++			     bool trusted)
+ {
+   struct grub_serial_port *p;
+   unsigned i;
+@@ -363,14 +364,23 @@ grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config
+ 	return &com_ports[i];
+       }
+ 
++  /*
++   * use_mmio and port share a union with the private data of every other back
++   * end, so they only mean anything on a port this driver registered.
++   */
+   FOR_SERIAL_PORTS (p)
+-    if (p->use_mmio == false && p->port == port)
++    if (p->driver == &grub_ns8250_driver
++	&& p->use_mmio == false && p->port == port)
+       {
+         if (config != NULL)
+           grub_serial_port_configure (p, config);
+         return p;
+       }
+ 
++  /* No port is registered here, so going on means probing the address. */
++  if (grub_serial_reject_untrusted_address (trusted) == true)
++    return NULL;
++
+   grub_outb (0x5a, port + UART_SR);
+   if (grub_inb (port + UART_SR) != 0x5a)
+     return NULL;
+@@ -402,7 +412,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_config *config
+ 
+ struct grub_serial_port *
+ grub_serial_ns8250_add_mmio (grub_addr_t addr, unsigned int acc_size,
+-                             struct grub_serial_config *config)
++                             struct grub_serial_config *config, bool trusted)
+ {
+   struct grub_serial_port *p;
+   unsigned i;
+@@ -415,14 +425,23 @@ grub_serial_ns8250_add_mmio (grub_addr_t addr, unsigned int acc_size,
+         return &com_ports[i];
+       }
+ 
++  /*
++   * use_mmio and mmio share a union with the private data of every other back
++   * end, so they only mean anything on a port this driver registered.
++   */
+   FOR_SERIAL_PORTS (p)
+-    if (p->use_mmio == true && p->mmio.base == addr)
++    if (p->driver == &grub_ns8250_driver
++	&& p->use_mmio == true && p->mmio.base == addr)
+       {
+         if (config != NULL)
+           grub_serial_port_configure (p, config);
+         return p;
+       }
+ 
++  /* No port is registered here, so going on means programming the address. */
++  if (grub_serial_reject_untrusted_address (trusted) == true)
++    return NULL;
++
+   p = grub_malloc (sizeof (*p));
+   if (p == NULL)
+     return NULL;
+diff --git a/grub-core/term/serial.c b/grub-core/term/serial.c
+index fe8501053..26e9d62f3 100644
+--- a/grub-core/term/serial.c
++++ b/grub-core/term/serial.c
+@@ -28,6 +28,7 @@
+ #include <grub/extcmd.h>
+ #include <grub/i18n.h>
+ #include <grub/list.h>
++#include <grub/lockdown.h>
+ #ifdef GRUB_MACHINE_MIPS_LOONGSON
+ #include <grub/machine/kernel.h>
+ #endif
+@@ -140,6 +141,30 @@ static struct grub_term_output grub_serial_term_output =
+ 
+ \f
+ 
++/*
++ * An address that no port is registered at cannot be honored while lockdown
++ * is enforced: the probe and programming sequences in the ns8250
++ * back ends write to fixed offsets from it with no way to tell a UART from
++ * ordinary memory, so an address taken from an unauthenticated configuration
++ * is a write primitive against whatever it points at.
++ *
++ * A back end calls this once its own lookup has found no port already
++ * registered at the address, that is, at the point where it would otherwise
++ * touch the hardware. TRUSTED is true when the address came from a platform
++ * description -- grub_ns8250_init(), an ACPI SPCR table, PCI enumeration --
++ * rather than from the caller.
++ */
++bool
++grub_serial_reject_untrusted_address (bool trusted)
++{
++  if (trusted == true || grub_is_lockdown () != GRUB_LOCKDOWN_ENABLED)
++    return false;
++
++  grub_error (GRUB_ERR_ACCESS_DENIED,
++	      N_("serial port address is not permitted when lockdown is enforced"));
++  return true;
++}
++
+ struct grub_serial_port *
+ grub_serial_find (const char *name)
+ {
+@@ -154,12 +179,18 @@ grub_serial_find (const char *name)
+     if (grub_strcmp (port->name, name) == 0)
+       return port;
+ 
++  /*
++   * Everything below this point builds a port from an address the caller
++   * gave us, so the back ends are told the address is untrusted: under
++   * lockdown they hand back a port only if one is already registered at that
++   * address, which means the platform described that UART.
++   */
+ #if (defined(__mips__) || defined (__i386__) || defined (__x86_64__)) && !defined(GRUB_MACHINE_EMU) && !defined(GRUB_MACHINE_ARC)
+   if (grub_strncmp (name, "port", sizeof ("port") - 1) == 0
+       && grub_isxdigit (name [sizeof ("port") - 1]))
+     {
+       port = grub_serial_ns8250_add_port (grub_strtoul (&name[sizeof ("port") - 1],
+-							0, 16), NULL);
++							0, 16), NULL, false);
+       if (port != NULL)
+         return port;
+     }
+@@ -205,7 +236,7 @@ grub_serial_find (const char *name)
+             grub_error (GRUB_ERR_BAD_ARGUMENT, N_("incorrect MMIO access size"));
+           }
+ 
+-      port = grub_serial_ns8250_add_mmio (addr, acc_size, NULL);
++      port = grub_serial_ns8250_add_mmio (addr, acc_size, NULL, false);
+       if (port != NULL)
+         return port;
+     }
+@@ -280,9 +311,18 @@ grub_cmd_serial (grub_extcmd_context_t ctxt, int argc, char **args)
+ 
+   port = grub_serial_find (name);
+   if (!port)
+-    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+-		       N_("serial port `%s' isn't found"),
+-		       name);
++    {
++      /*
++       * A back end that refused the port has already said why; keep that
++       * rather than reporting a misleading "isn't found".
++       */
++      if (grub_errno == GRUB_ERR_ACCESS_DENIED)
++	return grub_errno;
++
++      return grub_error (GRUB_ERR_BAD_ARGUMENT,
++			 N_("serial port `%s' isn't found"),
++			 name);
++    }
+ 
+   config = port->config;
+ 
+diff --git a/include/grub/serial.h b/include/grub/serial.h
+index 19cecd316..3c1b14d2b 100644
+--- a/include/grub/serial.h
++++ b/include/grub/serial.h
+@@ -188,14 +188,18 @@ grub_serial_config_defaults (struct grub_serial_port *port)
+   return port->driver->configure (port, &config);
+ }
+ 
++bool grub_serial_reject_untrusted_address (bool trusted);
++
+ #if defined(__mips__) || defined (__i386__) || defined (__x86_64__)
+ void grub_ns8250_init (void);
+ struct grub_serial_port *grub_ns8250_spcr_init (void);
+ struct grub_serial_port *grub_serial_ns8250_add_port (grub_port_t port,
+-						      struct grub_serial_config *config);
++						      struct grub_serial_config *config,
++						      bool trusted);
+ struct grub_serial_port *grub_serial_ns8250_add_mmio (grub_addr_t addr,
+ 						      unsigned int acc_size,
+-						      struct grub_serial_config *config);
++						      struct grub_serial_config *config,
++						      bool trusted);
+ #endif
+ #ifdef GRUB_MACHINE_IEEE1275
+ void grub_ofserial_init (void);

diff --git a/grub.patches b/grub.patches
index f16fe15..5cff4c8 100644
--- a/grub.patches
+++ b/grub.patches
@@ -442,4 +442,5 @@ Patch0447: 0447-tpm-Only-skip-TPM-string-PCR-measurements-with-expli.patch
 Patch0448: 0448-Revert-Use-medany-instead-of-large-model-for-RISCV.patch
 Patch0449: 0449-configure-Add-mno-relax-on-riscv.patch
 Patch0450: 0450-configure-Defer-check-for-mcmodel-large-until-PIC-PI.patch
-Patch0451: 0451-bli-Allow-overriding-PACKAGE_STRING-via-grub-mkimage.patch
\ No newline at end of file
+Patch0451: 0451-bli-Allow-overriding-PACKAGE_STRING-via-grub-mkimage.patch
+Patch0452: 0452-term-serial-Reject-caller-supplied-port-addresses-un.patch
\ No newline at end of file

diff --git a/grub2.spec b/grub2.spec
index c9e8ab3..33d16e6 100644
--- a/grub2.spec
+++ b/grub2.spec
@@ -17,7 +17,7 @@
 Name:		grub2
 Epoch:		1
 Version:	2.12
-Release:	78%{?dist}
+Release:	79%{?dist}
 Summary:	Bootloader with support for Linux, Multiboot and more
 License:	GPL-3.0-or-later
 URL:		http://www.gnu.org/software/grub/
@@ -702,6 +702,9 @@ fi
 %endif
 
 %changelog
+* Wed Sep 23 2026 Leo Sandoval <lsandova@redhat.com> - 2.12-79
+- term/serial: Reject caller-supplied port addresses under lockdown
+
 * Wed Sep 23 2026 Leo Sandoval <lsandova@redhat.com> - 2.12-78
 - bli: Allow overriding PACKAGE_STRING via grub-mkimage
 

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-09-24 15:47 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 15:47 [rpms/grub2] f45: term/serial: Reject caller-supplied port addresses under lockdown Leo Sandoval

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox