| Message ID | 20230822111321.56434-1-Hari.PrasathGE@microchip.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | serial: atmel: Fix Spectre v1 vulnerability reported by smatch | expand |
On Tue, Aug 22, 2023 at 04:43:21PM +0530, Hari Prasath Gujulan Elango wrote: > smatch reports the below spectre variant 1 vulnerability. > > drivers/tty/serial/atmel_serial.c:2675 atmel_console_setup() warn: potential spectre issue 'atmel_ports' [r] (local cap) > > Fix the same by using the array_index_nospec() to mitigate this > potential vulnerability especially because the console index is > controlled by user-space. > > Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com> > --- > drivers/tty/serial/atmel_serial.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c > index 3467a875641a..25f004dd9efd 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -33,6 +33,7 @@ > #include <linux/suspend.h> > #include <linux/mm.h> > #include <linux/io.h> > +#include <linux/nospec.h> > > #include <asm/div64.h> > #include <asm/ioctls.h> > @@ -2662,13 +2663,23 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud, > > static int __init atmel_console_setup(struct console *co, char *options) > { > - struct uart_port *port = &atmel_ports[co->index].uart; > - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); > + struct uart_port *port; > + struct atmel_uart_port *atmel_port; > int baud = 115200; > int bits = 8; > int parity = 'n'; > int flow = 'n'; > > + if (unlikely(co->index < 0 || co->index >= ATMEL_MAX_UART)) Only ever use likely/unlikely if you can measure the difference with and without the marking. Otherwise do not use it as the compiler and cpu do a better job than we do in figuring this out. > + return -ENODEV; > + > + co->index = array_index_nospec(co->index, ATMEL_MAX_UART); How exactl is index controlled by userspace such that a spectre gadget can be used here? You have to be able to call this multiple times in a row, unsuccessfully and successfully, how does that happen through the console api? thanks, greg k-h
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index 3467a875641a..25f004dd9efd 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -33,6 +33,7 @@ #include <linux/suspend.h> #include <linux/mm.h> #include <linux/io.h> +#include <linux/nospec.h> #include <asm/div64.h> #include <asm/ioctls.h> @@ -2662,13 +2663,23 @@ static void __init atmel_console_get_options(struct uart_port *port, int *baud, static int __init atmel_console_setup(struct console *co, char *options) { - struct uart_port *port = &atmel_ports[co->index].uart; - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port); + struct uart_port *port; + struct atmel_uart_port *atmel_port; int baud = 115200; int bits = 8; int parity = 'n'; int flow = 'n'; + if (unlikely(co->index < 0 || co->index >= ATMEL_MAX_UART)) + return -ENODEV; + + co->index = array_index_nospec(co->index, ATMEL_MAX_UART); + port = &atmel_ports[co->index].uart; + if (!port) + return -ENODEV; + + atmel_port = to_atmel_uart_port(port); + if (port->membase == NULL) { /* Port not initialized yet - delay setup */ return -ENODEV;
smatch reports the below spectre variant 1 vulnerability. drivers/tty/serial/atmel_serial.c:2675 atmel_console_setup() warn: potential spectre issue 'atmel_ports' [r] (local cap) Fix the same by using the array_index_nospec() to mitigate this potential vulnerability especially because the console index is controlled by user-space. Signed-off-by: Hari Prasath Gujulan Elango <Hari.PrasathGE@microchip.com> --- drivers/tty/serial/atmel_serial.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)