public inbox for git-commits@fedoraproject.org
help / color / mirror / Atom feed
* [rpms/grub2] rawhide: term/serial: Reject caller-supplied port addresses under lockdown
@ 2026-09-24 15:48 Leo Sandoval
0 siblings, 0 replies; only message in thread
From: Leo Sandoval @ 2026-09-24 15:48 UTC (permalink / raw)
To: git-commits
A new commit has been pushed.
Repo : rpms/grub2
Branch : rawhide
Commit : 61c18ff90b4e7134ead260f72ae24c73fe8ebf1b
Author : Leo Sandoval <lsandova@redhat.com>
Date : 2026-09-23T13:56:08-06:00
Stats : +255/-2 in 3 file(s)
URL : https://src.fedoraproject.org/rpms/grub2/c/61c18ff90b4e7134ead260f72ae24c73fe8ebf1b?branch=rawhide
Log:
term/serial: Reject caller-supplied port addresses under lockdown
Signed-off-by: Leo Sandoval <lsandova@redhat.com>
---
diff --git a/0453-term-serial-Reject-caller-supplied-port-addresses-un.patch b/0453-term-serial-Reject-caller-supplied-port-addresses-un.patch
new file mode 100644
index 0000000..91a8d21
--- /dev/null
+++ b/0453-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 302aa87..e1bfc7a 100644
--- a/grub.patches
+++ b/grub.patches
@@ -443,4 +443,5 @@ Patch0448: 0448-bli-Allow-overriding-PACKAGE_STRING-via-grub-mkimage.patch
Patch0449: 0449-Revert-Use-medany-instead-of-large-model-for-RISCV.patch
Patch0450: 0450-configure-Add-mno-relax-on-riscv.patch
Patch0451: 0451-configure-Defer-check-for-mcmodel-large-until-PIC-PI.patch
-Patch0452: 0452-platform-Return-i386-pc-when-proc-device-tree-is-pre.patch
\ No newline at end of file
+Patch0452: 0452-platform-Return-i386-pc-when-proc-device-tree-is-pre.patch
+Patch0453: 0453-term-serial-Reject-caller-supplied-port-addresses-un.patch
\ No newline at end of file
diff --git a/grub2.spec b/grub2.spec
index 7ba745a..69a1700 100644
--- a/grub2.spec
+++ b/grub2.spec
@@ -17,7 +17,7 @@
Name: grub2
Epoch: 1
Version: 2.12
-Release: 79%{?dist}
+Release: 80%{?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-80
+- term/serial: Reject caller-supplied port addresses under lockdown
+
* Tue Sep 01 2026 Leo Sandoval <lsandova@redhat.com> - 2.12-79
- platform: Return i386-pc when /proc/device-tree is present
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-09-24 15:48 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:48 [rpms/grub2] rawhide: 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