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]

 



Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> writes:

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

Maybe the intent would be clearer if you used bitwise operations instead
of arithmetic:

#define AU_MAP(x) (0x80 | (x))
[...]
	offset = au_io_in_map[offset];
	if (!(offset & 0x80))
		return UINT_MAX;
	offset &= 0x7f;
	return __raw_readl(...);

-- 
Måns Rullgård
mans@xxxxxxxxx
--
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