Re: [PATCH 1/2] serial: 8250: Validate reg addr for Au1x00/RT288x i/o accessors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/03/2015 18:31, Peter Hurley wrote:

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

Add the AU_MAP() macro to distinguish mapped and unmapped register
lookups (unmapped register lookups (initialized to 0) result in negative
register offsets). 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 | 41 ++++++++++++++++++++++---------------
  1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 5eb95fd..87df908 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -358,34 +358,43 @@ 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 */
+#define AU_MAP(x) (0x80 + x)
  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,
+	[UART_RX]  = AU_MAP(0),
+	[UART_IER] = AU_MAP(2),
+	[UART_IIR] = AU_MAP(3),
+	[UART_LCR] = AU_MAP(5),
+	[UART_MCR] = AU_MAP(6),
+	[UART_LSR] = AU_MAP(7),
+	[UART_MSR] = AU_MAP(8),
  };

  static const u8 au_io_out_map[] = {
-	[UART_TX]  = 1,
-	[UART_IER] = 2,
-	[UART_FCR] = 4,
-	[UART_LCR] = 5,
-	[UART_MCR] = 6,
+	[UART_TX]  = AU_MAP(1),
+	[UART_IER] = AU_MAP(2),
+	[UART_FCR] = AU_MAP(4),
+	[UART_LCR] = AU_MAP(5),
+	[UART_MCR] = AU_MAP(6),
  };

  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] - AU_MAP(0);
+	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] - AU_MAP(0);
+	if (offset < 0)
+		return;
+	__raw_writel(value, p->membase + (offset << p->regshift));
  }

Hello Peter,

I understand your concern about the lookup tables being sparse,
but once you've checked that they are not, why go to great length
to detect the issue at run-time?

Do you expect register offsets to change in the future?
(They've been stable for a very long time, according to git-log)

I would just go with "if (offset >= ARRAYSIZE(foo))" and not
the AU_MAP gymnastics.

On a tangential subject, as Mans stated, Au1x00 does not support
the scratch register; but Ralink and Tango do. These three
platforms share the weird register layout (requiring the LUTs),
but they differ in scratch support.

Would it make any sense to differentiate them, to support scratch
where it is implemented? (Although adding offset 7 to au_io_out_map
would then make that table sparse!)

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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux