Re: [PATCH 13/24] serial: 8250: dfl: validate MMIO resource size and regshift

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] serial: 8250: dfl: conflicting regshift check in dfl_uart_probe()
--

commit 01d97c927b977fd42973306928651fbf771b53aa
Author: Joshua Rogers <linux@joshua.hu>

serial: 8250: dfl: validate MMIO resource size and regshift

This commit adds validation in dfl_uart_probe() to check that the register
shift does not exceed 3 and that the MMIO resource size covers the full 8250
register window.

> diff --git a/drivers/tty/serial/8250/8250_dfl.c b/drivers/tty/serial/8250/8250_dfl.c
> index 72938990ea81e..e9329935bc476 100644
> --- a/drivers/tty/serial/8250/8250_dfl.c
> +++ b/drivers/tty/serial/8250/8250_dfl.c
> @@ -110,6 +110,7 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
>  {
>  	struct device *dev = &dfl_dev->dev;
>  	struct uart_8250_port uart = { };
> +	resource_size_t min_size;
>  	int ret;
>  
>  	uart.port.flags = UPF_IOREMAP;
> @@ -120,6 +121,17 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "failed uart feature walk\n");
>  
> +	if (uart.port.regshift > 3)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "unsupported regshift %u\n",
> +				     uart.port.regshift);

[Severity: Low]
Just before this check, dfl_uart_probe() calls dfl_uart_get_params(), which
extracts uart->port.regshift from DFHv1_PARAM_ID_REG_LAYOUT and already
validates it against a different upper bound:

drivers/tty/serial/8250/8250_dfl.c:dfl_uart_get_params() {
	...
	uart->port.regshift = FIELD_GET(DFHv1_PARAM_REG_LAYOUT_SHIFT, reg_layout);
	if (uart->port.regshift > 4)
		return dev_err_probe(dev, -EINVAL, "unsupported reg-shift %u\n",
				     uart->port.regshift);
	...
}

When DFHv1_PARAM_ID_REG_LAYOUT specifies a regshift of 4, it is accepted by
dfl_uart_get_params() only to be rejected immediately after in
dfl_uart_probe().

Should the bound in dfl_uart_get_params() be updated to 3 instead of adding a
second conflicting check in dfl_uart_probe()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-tty-llm-v1-0-99b0202ae546@linuxfoundation.org?part=13