| Message ID | 20170518112955.12235-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, May 18, 2017 at 12:29:55PM +0100, Ard Biesheuvel wrote: > Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") > modified the probing logic for PNP0501 devices, to remove a collision > between the generic 16550A driver and the Fintek driver, which reused > the same ACPI _HID. > > The Fintek device probe is now incorporated into the common 8250 probe > path, and gets called for all discovered 16550A compatible devices, > including ones that are MMIO mapped rather than IO mapped. However, > the Fintek driver assumes the port base is a I/O address, and proceeds > to probe some arbitrary offsets above it. > > This is generally a wrong thing to do, but on ARM systems (having no > native port I/O), this may result in faulting accesses of completely > unrelated MMIO regions in the PCI I/O space. Given that this is at > serial probe time, this results in hard to diagnose crashes at boot. > > So let's restrict the Fintek probe to devices that we know are using > port I/O in the first place. > > Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > Resending with Ricardo's ack added. > > Could we please tag this for -stable? Thanks. You could have done that in the patch itself, why have me manually add it? -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 May 2017 at 12:49, Greg KH <gregkh@linuxfoundation.org> wrote: > On Thu, May 18, 2017 at 12:29:55PM +0100, Ard Biesheuvel wrote: >> Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") >> modified the probing logic for PNP0501 devices, to remove a collision >> between the generic 16550A driver and the Fintek driver, which reused >> the same ACPI _HID. >> >> The Fintek device probe is now incorporated into the common 8250 probe >> path, and gets called for all discovered 16550A compatible devices, >> including ones that are MMIO mapped rather than IO mapped. However, >> the Fintek driver assumes the port base is a I/O address, and proceeds >> to probe some arbitrary offsets above it. >> >> This is generally a wrong thing to do, but on ARM systems (having no >> native port I/O), this may result in faulting accesses of completely >> unrelated MMIO regions in the PCI I/O space. Given that this is at >> serial probe time, this results in hard to diagnose crashes at boot. >> >> So let's restrict the Fintek probe to devices that we know are using >> port I/O in the first place. >> >> Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") >> Suggested-by: Arnd Bergmann <arnd@arndb.de> >> Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> Resending with Ricardo's ack added. >> >> Could we please tag this for -stable? Thanks. > > You could have done that in the patch itself, why have me manually add > it? > Because I think it is part of the review/discussion whether it should be tagged for stable or not, and it is ultimately up to the maintainer that applies it whether the tag is added to the patch. ---->8----- Cc: <stable@vger.kernel.org> ---->8----- -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c index 09a65a3ec7f7..e7765f010fe8 100644 --- a/drivers/tty/serial/8250/8250_port.c +++ b/drivers/tty/serial/8250/8250_port.c @@ -1337,7 +1337,7 @@ static void autoconfig(struct uart_8250_port *up) /* * Check if the device is a Fintek F81216A */ - if (port->type == PORT_16550A) + if (port->type == PORT_16550A && port->iotype == UPIO_PORT) fintek_8250_probe(up); if (up->capabilities != old_capabilities) {
Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") modified the probing logic for PNP0501 devices, to remove a collision between the generic 16550A driver and the Fintek driver, which reused the same ACPI _HID. The Fintek device probe is now incorporated into the common 8250 probe path, and gets called for all discovered 16550A compatible devices, including ones that are MMIO mapped rather than IO mapped. However, the Fintek driver assumes the port base is a I/O address, and proceeds to probe some arbitrary offsets above it. This is generally a wrong thing to do, but on ARM systems (having no native port I/O), this may result in faulting accesses of completely unrelated MMIO regions in the PCI I/O space. Given that this is at serial probe time, this results in hard to diagnose crashes at boot. So let's restrict the Fintek probe to devices that we know are using port I/O in the first place. Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base") Suggested-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Resending with Ricardo's ack added. Could we please tag this for -stable? Thanks. drivers/tty/serial/8250/8250_port.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html