Re: [PATCH RESEND v4] ACPI: SPCR: Support UART clock frequency field

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] acpi: spcr: out-of-bounds read of uart_clk_freq
--

commit 4417c1e28359b400b5ff86436180ed471e8b7aca
Author: Markus Probst <markus.probst@posteo.de>

ACPI: SPCR: Support UART clock frequency field

This commit adds support for the UART Clock Frequency field from the Microsoft
SPCR specification revision 1.08. If present, it uses the field to configure
the serial port with the correct uart clock frequency.

Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/serports/serial-port-console-redirection-table

> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -228,7 +228,8 @@ int __init acpi_parse_spcr(bool enable_earlycon, bool enable_console)
>  	pr_info("console: %s\n", opts);
>  
>  	if (enable_earlycon)
> -		setup_earlycon(opts);
> +		setup_earlycon_with_uartclk(opts,
> +					    table->header.revision >= 3 ? table->uart_clk_freq : 0);

[Severity: Medium]
Could this result in an out-of-bounds memory read?

In acpi_parse_spcr(), the code accesses table->uart_clk_freq based solely on
table->header.revision >= 3 without validating that the mapped table length
(table->header.length) actually covers the uart_clk_freq field.

If a malformed ACPI SPCR table has a high revision but a length shorter than
the required size (at least 80 bytes for uart_clk_freq), the access could read
past the bounds of the table mapped by acpi_get_table(). This could
potentially lead to a kernel panic if it crosses an unmapped page boundary.

Should this also check table->header.length before accessing uart_clk_freq?

>  
>  	if (enable_console)
>  		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-acpi_spcr-v4-1-f9a96fe016af@posteo.de?part=1