Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Maciej W. Rozycki]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> pci_oxsemi_tornado_init() reads the UART count from device-controlled
> BAR0 memory without validation, allowing a malicious device to report up
> to 255 ports. Cap the device-reported count against what actually fits
> in the BAR using the Tornado register layout.

 Hmm, what is "a malicious device" supposed to be, a broken chip?

> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 1dda63be6b89..9919504334bb 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>  	/* Tornado device */
>  	if (deviceID == 0x07000200) {
>  		number_uarts = ioread8(p + 4);
> +
> +		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
> +		bar_len = pci_resource_len(dev, 0);
> +		if (bar_len >= 0x1000)
> +			number_uarts = min_t(unsigned int, number_uarts,
> +					     (bar_len - 0x1000) / 0x200);
> +		else
> +			number_uarts = 0;
> +

 Technically an overkill and duplicating the FL_REGION_SZ_CAP feature 
rather than making use of it, and then falsifying the actual number of 
ports report in the kernel log (which I think ought to be accurate even if 
do clamp the number).

 Sashiko is however right that this function ought to return an error 
rather than 0, and also should BAR0 be smaller than 8, we already violate 
access with the ioread8() call quoted (and possibly ioread32() earlier 
on).

 So I think overall this ought to look like (unchecked):

static int pci_oxsemi_tornado_init(struct pci_dev *dev)
{
	u8 __iomem *p;
	unsigned long deviceID;
	int number_uarts = -ENXIO;

	if (!pci_oxsemi_tornado_p(dev) || pci_resource_len(dev, 0) < 8)
		return -ENXIO;

	p = pci_iomap(dev, 0, 5);
	if (p == NULL)
		return -ENOMEM;

	deviceID = ioread32(p);
	/* Tornado device */
	if (deviceID == 0x07000200) {
		number_uarts = ioread8(p + 4);
		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
			number_uarts,
			dev->vendor == PCI_VENDOR_ID_ENDRUN ?
			"EndRun" : "Oxford");
		if (number_uarts < 1)
			number_uarts = -ENXIO;
	}
	pci_iounmap(dev, p);
	return number_uarts;
}

with the FL_REGION_SZ_CAP flag set for the relevant entries, and possibly 
setting ->num_ports to 0 too since we need to determine that at run time.

 Not sure how this interferes with Brainboxes devices, support for which 
seems not to have been fully merged given that pci_oxsemi_tornado_p() does 
not let PCI_VENDOR_ID_INTASHIELD devices through.  Cameron?

 NB BAR0 size is 16k by the spec and the maximum # of UARTs is 16, for a 
dual-OXPCIe958 master/slave setup, still leaving 4k of the available space 
unoccupied.

  Maciej