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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Oct 27, 2026 at 02:29:21PM +0000, Maciej W. Rozycki wrote:
> 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?

Carefully reconstructed malicious device in FPGA that gets connected via
Thunderbolt. This is real vector of attacks (assuming the Thunderbolt side is
misconfigured to automatically authorise anything connected to it).

IRL I don't believe it would ever happen as the result of this attack most
likely would be minor if any.

...

> 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);

Why not 8 instead of 5?

> 	if (p == NULL)
> 		return -ENOMEM;

This is so interesting... For MMIO it gives a page, I don't see how it can go
outside the boundaries. For port based IO I think it can't be less than 4 for
sure (due to PCI bits to interpret) and most likely can't be less than 8. But
I have a weak memory of PCI specifications.

> 	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?

-- 
With Best Regards,
Andy Shevchenko