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

[Markus Probst <markus.probst@posteo.de>]

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

On Wed, 2026-09-30 at 15:45 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] acpi: out-of-bounds read of SPCR uart_clk_freq field
> --
> 
> commit 541ea063ef44d64e511d2deae13a8adcab45ebc7
> Author: Markus Probst <markus.probst@posteo.de>
> 
> ACPI: SPCR: Support UART clock frequency field
> 
> This commit adds support for the UART Clock Frequency field introduced in the
> Microsoft SPCR specification revision 1.08. It introduces a new function
> setup_earlycon_with_uartclk() to configure the serial port with the correct
> clock frequency from the SPCR table if available.
> 
> 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
> > index 73cb933fdc89b..c79c809f49d46 100644
> > --- 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]
> Does this code risk an out-of-bounds memory read when accessing
> table->uart_clk_freq?
> 
> In acpi_parse_spcr(), the table is mapped using acpi_get_table(), which
> only maps the number of bytes specified by table->header.length.
> 
> If a malformed ACPI SPCR table is provided by system firmware where
> table->header.revision >= 3 but the length is too small to contain
> uart_clk_freq, could this access read past the mapped length?
> 
> This could result in unmapped memory accesses leading to an early boot kernel
> panic, or silently reading garbage data which results in an incorrect early
> console UART clock frequency.
> 
> Should there be a validation of table->header.length against the expected
> size containing the uart_clk_freq field before accessing it?
> 
> >  
> >  	if (enable_console)
> >  		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
Fields from revision 4 are being accessed, so I don't think accessing
one from revision 3 requires more checking.




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]