Peter Hurley wrote:
Peter Hurley wrote:
Perhaps a better way to do this would be to explicitly declare the
table sizes for the complete 8250 register set and specifically
initialize the unmapped registers to -1.
That would do away with AU_MAP() and arithmetic, while it would still
make it more obvious that unmapped registers need specific initialization
and would cause compile errors if an initializer extended the register
map.
FWIW, that patch would look like this:
--- >% ---
From: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
Subject: [PATCH] serial: 8250: Validate reg addr for Au1x00/RT288x i/o
accessors
Au1x00/RT2800+ hardware has an alternate register layout which is
remapped with lookup tables by the au_serial_in()/out() i/o accessors.
However, the h/w does not support the complete 8250 register set, and
accesses to unmapped registers cause out-of-bounds lookups. Further,
because the lookup tables are defined by designated initializers, the
tables may contain unmapped entries (although the current tables do not).
Declare fixed-size lookup tables with contiguous initialization for
the complete 8250 register map; unmapped registers are initialized to -1.
Validate the register index (ie., 'offset') is in the range [0, table size).
Return fixed value for unmapped register reads and ignore unmapped register
writes.
Reported-by: Mason <slash.tmp@xxxxxxx>
Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
drivers/tty/serial/8250/8250_core.c | 48 +++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 5eb95fd..bfff624 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -358,34 +358,46 @@ static void default_serial_dl_write(struct uart_8250_port *up, int value)
#if defined(CONFIG_MIPS_ALCHEMY) || defined(CONFIG_SERIAL_8250_RT288X)
/* Au1x00/RT288x UART hardware has a weird register layout */
-static const u8 au_io_in_map[] = {
- [UART_RX] = 0,
- [UART_IER] = 2,
- [UART_IIR] = 3,
- [UART_LCR] = 5,
- [UART_MCR] = 6,
- [UART_LSR] = 7,
- [UART_MSR] = 8,
+static const s8 au_io_in_map[8] = {
+ 0, /* UART_RX */
+ 2, /* UART_IER */
+ 3, /* UART_IIR */
+ 5, /* UART_LCR */
+ 6, /* UART_MCR */
+ 7, /* UART_LSR */
+ 8, /* UART_MSR */
+ -1, /* UART_SCR (unmapped) */
};
-static const u8 au_io_out_map[] = {
- [UART_TX] = 1,
- [UART_IER] = 2,
- [UART_FCR] = 4,
- [UART_LCR] = 5,
- [UART_MCR] = 6,
+static const s8 au_io_out_map[8] = {
+ 1, /* UART_TX */
+ 2, /* UART_IER */
+ 4, /* UART_FCR */
+ 5, /* UART_LCR */
+ 6, /* UART_MCR */
+ -1, /* UART_LSR (unmapped) */
+ -1, /* UART_MSR (unmapped) */
+ -1, /* UART_SCR (unmapped) */
};
static unsigned int au_serial_in(struct uart_port *p, int offset)
{
- offset = au_io_in_map[offset] << p->regshift;
- return __raw_readl(p->membase + offset);
+ if (offset >= ARRAYSIZE(au_io_in_map))
+ return UINT_MAX;
+ offset = au_io_in_map[offset];
+ if (offset < 0)
+ return UINT_MAX;
+ return __raw_readl(p->membase + (offset << p->regshift));
}
static void au_serial_out(struct uart_port *p, int offset, int value)
{
- offset = au_io_out_map[offset] << p->regshift;
- __raw_writel(value, p->membase + offset);
+ if (offset >= ARRAYSIZE(au_io_out_map))
+ return;
+ offset = au_io_out_map[offset];
+ if (offset < 0)
+ return;
+ __raw_writel(value, p->membase + (offset << p->regshift));
}
Agreed, it does look better.
And the LUTs come out to an even 64 bits :-)
Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
