* [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases
@ 2018-02-20 9:40 Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias Geert Uytterhoeven
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
Hi all,
Serial drivers used on DT platforms use the "serialN" alias in DT to
obtain the serial port index for a specific port. Drivers typically use
a fixed-size array for keeping track of all available serial ports.
However, several drivers do not perform any validation on the index
obtained from DT, which may lead to out-of-bounds accesses of these
fixed-size arrays.
While the DTB passed to the kernel might be considered trusted, some of
these out-of-bounds accesses can be triggered by a legitimate DTB:
- In some drivers the size of the array is defined by a Kconfig
symbol, so a user who doesn't need all serial ports may lower this
value rightfully,
- Tomorrow's new SoC may have more serial ports than the fixed-size
array in today's driver can accommodate, which the user may forget
to enlarge.
Hence this series fixes that by adding checks for out-of-range aliases,
logging an error message when triggered.
Tested on r8a7791/koelsch (sh-sci), all other drivers were
compile-tested only.
Thanks for your comments!
Geert Uytterhoeven (9):
serial: arc_uart: Fix out-of-bounds access through DT alias
serial: fsl_lpuart: Fix out-of-bounds access through DT alias
serial: imx: Fix out-of-bounds access through DT alias
serial: mxs-auart: Fix out-of-bounds access through DT alias
serial: pxa: Fix out-of-bounds access through DT alias
serial: samsung: Fix out-of-bounds access through DT alias
serial: sh-sci: Fix out-of-bounds access through DT alias
serial: sirf: Fix out-of-bounds access through DT alias
serial: xuartps: Fix out-of-bounds access through DT alias
drivers/tty/serial/arc_uart.c | 5 +++++
drivers/tty/serial/fsl_lpuart.c | 4 ++++
drivers/tty/serial/imx.c | 5 +++++
drivers/tty/serial/mxs-auart.c | 4 ++++
drivers/tty/serial/pxa.c | 4 ++++
drivers/tty/serial/samsung.c | 4 ++++
drivers/tty/serial/sh-sci.c | 4 ++++
drivers/tty/serial/sirfsoc_uart.c | 5 +++++
drivers/tty/serial/xilinx_uartps.c | 2 +-
9 files changed, 36 insertions(+), 1 deletion(-)
--
2.7.4
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:50 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 2/9] serial: fsl_lpuart: " Geert Uytterhoeven
` (7 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The arc_uart_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_ARC_NR_PORTS), so this can even be triggered using a
legitimate DTB.
Fixes: 10640deb04b7949a ("serial: arc_uart: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/arc_uart.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/arc_uart.c b/drivers/tty/serial/arc_uart.c
index 2599f9ecccfe7769..1cb827a6b836d0dd 100644
--- a/drivers/tty/serial/arc_uart.c
+++ b/drivers/tty/serial/arc_uart.c
@@ -593,6 +593,11 @@ static int arc_serial_probe(struct platform_device *pdev)
if (dev_id < 0)
dev_id = 0;
+ if (dev_id >= CONFIG_SERIAL_ARC_NR_PORTS) {
+ dev_err(&pdev->dev, "serial%d out of range\n", dev_id);
+ return -EINVAL;
+ }
+
uart = &arc_uart_ports[dev_id];
port = &uart->port;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/9] serial: fsl_lpuart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:51 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 3/9] serial: imx: " Geert Uytterhoeven
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The lpuart_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: 970416c691dc68b5 ("serial: fsl_lpuart: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/fsl_lpuart.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 8cf112f2efc30707..5218e2ed1ba90a5b 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -2145,6 +2145,10 @@ static int lpuart_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "failed to get alias id, errno %d\n", ret);
return ret;
}
+ if (ret >= UART_NR) {
+ dev_err(&pdev->dev, "serial%d out of range\n", ret);
+ return -EINVAL;
+ }
sport->port.line = ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
sport->port.membase = devm_ioremap_resource(&pdev->dev, res);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 2/9] serial: fsl_lpuart: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:31 ` Uwe Kleine-König
2018-02-20 9:40 ` [PATCH 4/9] serial: mxs-auart: " Geert Uytterhoeven
` (5 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The imx_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: 9206ab8a0350c3da ("serial: imx: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/imx.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 1d7ca382bc12b238..e89e90ad87d8245c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2041,6 +2041,11 @@ static int serial_imx_probe(struct platform_device *pdev)
serial_imx_probe_pdata(sport, pdev);
else if (ret < 0)
return ret;
+ if (sport->port.line >= UART_NR) {
+ dev_err(&pdev->dev, "serial%d out of range\n",
+ sport->port.line);
+ return -EINVAL;
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(&pdev->dev, res);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] serial: mxs-auart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (2 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 3/9] serial: imx: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:51 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 5/9] serial: pxa: " Geert Uytterhoeven
` (4 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The auart_port[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: cabf23e7aa00b145 ("serial: mxs-auart: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/mxs-auart.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 079dc47aa142d8e1..7ec1298512facfc8 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -1663,6 +1663,10 @@ static int mxs_auart_probe(struct platform_device *pdev)
s->port.line = pdev->id < 0 ? 0 : pdev->id;
else if (ret < 0)
return ret;
+ if (s->port.line >= MXS_AUART_PORTS) {
+ dev_err(&pdev->dev, "serial%d out of range\n", s->port.line);
+ return -EINVAL;
+ }
if (of_id) {
pdev->id_entry = of_id->data;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] serial: pxa: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (3 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 4/9] serial: mxs-auart: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:52 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 6/9] serial: samsung: " Geert Uytterhoeven
` (3 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The serial_pxa_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: c8dcdc77298dde67 ("serial: pxa: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/pxa.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index baf552944d5686e8..ac25faa95baaad0e 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -885,6 +885,10 @@ static int serial_pxa_probe(struct platform_device *dev)
sport->port.line = dev->id;
else if (ret < 0)
goto err_clk;
+ if (sport->port.line > ARRAY_SIZE(serial_pxa_ports)) {
+ dev_err(&dev->dev, "serial%d out of range\n", sport->port.line);
+ return -EINVAL;
+ }
snprintf(sport->name, PXA_NAME_LEN - 1, "UART%d", sport->port.line + 1);
sport->port.membase = ioremap(mmres->start, resource_size(mmres));
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/9] serial: samsung: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (4 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 5/9] serial: pxa: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:52 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 7/9] serial: sh-sci: " Geert Uytterhoeven
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The s3c24xx_serial_ports[] array is indexed using a value derived from
the "serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_SAMSUNG_UARTS), so this can even be triggered using a
legitimate DTB.
Fixes: 3ac337e76a1c637b ("serial: samsung: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/samsung.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index f9fecc5ed0cee826..9ea42197ceefabba 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -1818,6 +1818,10 @@ static int s3c24xx_serial_probe(struct platform_device *pdev)
dbg("s3c24xx_serial_probe(%p) %d\n", pdev, index);
+ if (index >= CONFIG_SERIAL_SAMSUNG_UARTS) {
+ dev_err(&pdev->dev, "serial%d out of range\n", index);
+ return -EINVAL;
+ }
ourport = &s3c24xx_serial_ports[index];
ourport->drv_data = s3c24xx_get_driver_data(pdev);
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] serial: sh-sci: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (5 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 6/9] serial: samsung: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:53 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 8/9] serial: sirf: " Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 9/9] serial: xuartps: " Geert Uytterhoeven
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The sci_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Note that the array size is defined by a Kconfig symbol
(CONFIG_SERIAL_SH_SCI_NR_UARTS), so this can even be triggered using a
legitimate DTB.
Fixes: f650cdf1c115498e ("serial: sh-sci: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/sh-sci.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 4d14f321cbec95e0..0fb4784860da6188 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -3620,6 +3620,10 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
dev_err(&pdev->dev, "failed to get alias id (%d)\n", id);
return NULL;
}
+ if (id >= SCI_NPORTS) {
+ dev_err(&pdev->dev, "serial%d out of range\n", id);
+ return NULL;
+ }
sp = &sci_ports[id];
*dev_id = id;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] serial: sirf: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (6 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 7/9] serial: sh-sci: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:53 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 9/9] serial: xuartps: " Geert Uytterhoeven
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The sirf_ports[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: 66c7ab1120585d18 ("serial: sirf: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/sirfsoc_uart.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/sirfsoc_uart.c b/drivers/tty/serial/sirfsoc_uart.c
index 9925b00a97772a1b..701df3319ff7579d 100644
--- a/drivers/tty/serial/sirfsoc_uart.c
+++ b/drivers/tty/serial/sirfsoc_uart.c
@@ -1283,6 +1283,11 @@ static int sirfsoc_uart_probe(struct platform_device *pdev)
goto err;
}
sirfport->port.line = of_alias_get_id(np, "serial");
+ if (sirfport->port.line >= SIRFSOC_UART_NR) {
+ dev_err(&pdev->dev, "serial%d out of range\n",
+ sirfport->port.line);
+ return -EINVAL;
+ }
sirf_ports[sirfport->port.line] = sirfport;
sirfport->port.iotype = UPIO_MEM;
sirfport->port.flags = UPF_BOOT_AUTOCONF;
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
` (7 preceding siblings ...)
2018-02-20 9:40 ` [PATCH 8/9] serial: sirf: " Geert Uytterhoeven
@ 2018-02-20 9:40 ` Geert Uytterhoeven
2018-02-20 10:22 ` Michal Simek
8 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 9:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Barry Song, Geert Uytterhoeven, Vineet Gupta,
Michal Simek, linux-kernel, linux-renesas-soc, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
The cdns_uart_port[] array is indexed using a value derived from the
"serialN" alias in DT, which may lead to an out-of-bounds access.
Fix this by adding a range check.
Fixes: 1f118c02a1819856 ("serial: xuartps: Fix out-of-bounds access through DT alias")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
drivers/tty/serial/xilinx_uartps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
struct uart_port *port;
/* Try the given port id if failed use default method */
- if (cdns_uart_port[id].mapbase != 0) {
+ if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
/* Find the next unused port */
for (id = 0; id < CDNS_UART_NR_PORTS; id++)
if (cdns_uart_port[id].mapbase == 0)
--
2.7.4
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 9/9] serial: xuartps: " Geert Uytterhoeven
@ 2018-02-20 10:22 ` Michal Simek
2018-02-20 10:38 ` Geert Uytterhoeven
0 siblings, 1 reply; 24+ messages in thread
From: Michal Simek @ 2018-02-20 10:22 UTC (permalink / raw)
To: Geert Uytterhoeven, Greg Kroah-Hartman
Cc: devicetree, Barry Song, Vineet Gupta, Michal Simek, linux-kernel,
linux-renesas-soc, linux-serial, Jiri Slaby, linux-snps-arc,
linux-arm-kernel
On 20.2.2018 10:40, Geert Uytterhoeven wrote:
> The cdns_uart_port[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: 1f118c02a1819856 ("serial: xuartps: Fix out-of-bounds access through DT alias")
I didn't find this sha1 - patch name is this one.
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/tty/serial/xilinx_uartps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
> struct uart_port *port;
>
> /* Try the given port id if failed use default method */
> - if (cdns_uart_port[id].mapbase != 0) {
> + if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
> /* Find the next unused port */
> for (id = 0; id < CDNS_UART_NR_PORTS; id++)
> if (cdns_uart_port[id].mapbase == 0)
>
Below should be better fix for this driver.
Thanks,
Michal
diff --git a/drivers/tty/serial/xilinx_uartps.c
b/drivers/tty/serial/xilinx_uartps.c
index b9b2bc76bcac..b77c6477ed93 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1109,6 +1109,9 @@ static struct uart_port *cdns_uart_get_port(int id)
{
struct uart_port *port;
+ if (id >= CDNS_UART_NR_PORTS)
+ return NULL;
+
/* Try the given port id if failed use default method */
if (cdns_uart_port[id].mapbase != 0) {
/* Find the next unused port */
@@ -1117,9 +1120,6 @@ static struct uart_port *cdns_uart_get_port(int id)
break;
}
- if (id >= CDNS_UART_NR_PORTS)
- return NULL;
-
port = &cdns_uart_port[id];
/* At this point, we've got an empty uart_port struct,
initialize it */
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 3/9] serial: imx: " Geert Uytterhoeven
@ 2018-02-20 10:31 ` Uwe Kleine-König
2018-02-20 10:49 ` Geert Uytterhoeven
0 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2018-02-20 10:31 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: devicetree, Barry Song, Greg Kroah-Hartman, Michal Simek,
linux-kernel, linux-renesas-soc, Vineet Gupta, linux-serial,
Jiri Slaby, linux-snps-arc, linux-arm-kernel
Hello Geert,
On Tue, Feb 20, 2018 at 10:40:18AM +0100, Geert Uytterhoeven wrote:
> The imx_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: 9206ab8a0350c3da ("serial: imx: Fix out-of-bounds access through DT alias")
huh, this patch fixes itself?
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> drivers/tty/serial/imx.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 1d7ca382bc12b238..e89e90ad87d8245c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -2041,6 +2041,11 @@ static int serial_imx_probe(struct platform_device *pdev)
> serial_imx_probe_pdata(sport, pdev);
> else if (ret < 0)
> return ret;
I'd prefer an empty line here.
> + if (sport->port.line >= UART_NR) {
I would have used:
if (sport->port.line >= ARRAY_SIZE(imx_ports))
which IMHO is better understandable
> + dev_err(&pdev->dev, "serial%d out of range\n",
> + sport->port.line);
Note that the same overflow can happen when a device is probed using
platform data (and your commit fixes that, too). Maybe worth to point
out in the commit log?
Other than that: Good catch, thanks for your patch.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 10:22 ` Michal Simek
@ 2018-02-20 10:38 ` Geert Uytterhoeven
2018-02-20 11:27 ` Michal Simek
0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:38 UTC (permalink / raw)
To: Michal Simek
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Geert Uytterhoeven, Greg Kroah-Hartman,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
Hi Michal,
On Tue, Feb 20, 2018 at 11:22 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 20.2.2018 10:40, Geert Uytterhoeven wrote:
>> The cdns_uart_port[] array is indexed using a value derived from the
>> "serialN" alias in DT, which may lead to an out-of-bounds access.
>>
>> Fix this by adding a range check.
>>
>> Fixes: 1f118c02a1819856 ("serial: xuartps: Fix out-of-bounds access through DT alias")
>
> I didn't find this sha1 - patch name is this one.
Bummer, I totally screwed up my scripting...
Fixes: 928e9263492069ee ("tty: xuartps: Initialize ports according to aliases")
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> drivers/tty/serial/xilinx_uartps.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
>> index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
>> --- a/drivers/tty/serial/xilinx_uartps.c
>> +++ b/drivers/tty/serial/xilinx_uartps.c
>> @@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
>> struct uart_port *port;
>>
>> /* Try the given port id if failed use default method */
>> - if (cdns_uart_port[id].mapbase != 0) {
>> + if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
>> /* Find the next unused port */
>> for (id = 0; id < CDNS_UART_NR_PORTS; id++)
>> if (cdns_uart_port[id].mapbase == 0)
>>
>
> Below should be better fix for this driver.
I considered that, too, but...
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1109,6 +1109,9 @@ static struct uart_port *cdns_uart_get_port(int id)
> {
> struct uart_port *port;
>
> + if (id >= CDNS_UART_NR_PORTS)
> + return NULL;
> +
> /* Try the given port id if failed use default method */
> if (cdns_uart_port[id].mapbase != 0) {
> /* Find the next unused port */
> @@ -1117,9 +1120,6 @@ static struct uart_port *cdns_uart_get_port(int id)
> break;
> }
>
> - if (id >= CDNS_UART_NR_PORTS)
> - return NULL;
> -
... the above check cannot be removed, as it is needed to support the loop
above to find an unused port.
> port = &cdns_uart_port[id];
>
> /* At this point, we've got an empty uart_port struct,
> initialize it */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
2018-02-20 10:31 ` Uwe Kleine-König
@ 2018-02-20 10:49 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:49 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Geert Uytterhoeven, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
Hi Uwe,
On Tue, Feb 20, 2018 at 11:31 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Tue, Feb 20, 2018 at 10:40:18AM +0100, Geert Uytterhoeven wrote:
>> The imx_ports[] array is indexed using a value derived from the
>> "serialN" alias in DT, which may lead to an out-of-bounds access.
>>
>> Fix this by adding a range check.
>>
>> Fixes: 9206ab8a0350c3da ("serial: imx: Fix out-of-bounds access through DT alias")
>
> huh, this patch fixes itself?
Oops
Fixes: ff05967a07225ab6 ("serial/imx: add of_alias_get_id() reference back")
>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> drivers/tty/serial/imx.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 1d7ca382bc12b238..e89e90ad87d8245c 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -2041,6 +2041,11 @@ static int serial_imx_probe(struct platform_device *pdev)
>> serial_imx_probe_pdata(sport, pdev);
>> else if (ret < 0)
>> return ret;
>
> I'd prefer an empty line here.
OK
>> + if (sport->port.line >= UART_NR) {
>
> I would have used:
>
> if (sport->port.line >= ARRAY_SIZE(imx_ports))
>
> which IMHO is better understandable
OK.
>> + dev_err(&pdev->dev, "serial%d out of range\n",
>> + sport->port.line);
>
> Note that the same overflow can happen when a device is probed using
> platform data (and your commit fixes that, too). Maybe worth to point
> out in the commit log?
That's correct. But board code is tied more intimate to the kernel.
Will update.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias Geert Uytterhoeven
@ 2018-02-20 10:50 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:50 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The arc_uart_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Note that the array size is defined by a Kconfig symbol
> (CONFIG_SERIAL_ARC_NR_PORTS), so this can even be triggered using a
> legitimate DTB.
>
> Fixes: 10640deb04b7949a ("serial: arc_uart: Fix out-of-bounds access through DT alias")
Fixes: ea28fd56fcde69af ("serial/arc-uart: switch to devicetree based probing")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/9] serial: fsl_lpuart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 2/9] serial: fsl_lpuart: " Geert Uytterhoeven
@ 2018-02-20 10:51 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The lpuart_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: 970416c691dc68b5 ("serial: fsl_lpuart: Fix out-of-bounds access through DT alias")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Fixes: c9e2e946fb0ba5d2 ("tty: serial: add Freescale lpuart driver support")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/9] serial: mxs-auart: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 4/9] serial: mxs-auart: " Geert Uytterhoeven
@ 2018-02-20 10:51 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The auart_port[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: cabf23e7aa00b145 ("serial: mxs-auart: Fix out-of-bounds access through DT alias")
Fixes: 1ea6607d4cdc9179 ("serial: mxs-auart: Allow device tree probing")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 5/9] serial: pxa: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 5/9] serial: pxa: " Geert Uytterhoeven
@ 2018-02-20 10:52 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The serial_pxa_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: c8dcdc77298dde67 ("serial: pxa: Fix out-of-bounds access through DT alias")
Fixes: 699c20f3e6310aa2 ("serial: pxa: add OF support")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/9] serial: samsung: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 6/9] serial: samsung: " Geert Uytterhoeven
@ 2018-02-20 10:52 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:52 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The s3c24xx_serial_ports[] array is indexed using a value derived from
> the "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Note that the array size is defined by a Kconfig symbol
> (CONFIG_SERIAL_SAMSUNG_UARTS), so this can even be triggered using a
> legitimate DTB.
>
> Fixes: 3ac337e76a1c637b ("serial: samsung: Fix out-of-bounds access through DT alias")
Fixes: 13a9f6c64fdc55eb ("serial: samsung: Consider DT alias when probing po
rts")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 7/9] serial: sh-sci: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 7/9] serial: sh-sci: " Geert Uytterhoeven
@ 2018-02-20 10:53 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:53 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The sci_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Note that the array size is defined by a Kconfig symbol
> (CONFIG_SERIAL_SH_SCI_NR_UARTS), so this can even be triggered using a
> legitimate DTB.
>
> Fixes: f650cdf1c115498e ("serial: sh-sci: Fix out-of-bounds access through DT alias")
Fixes: 97ed9790c514066b ("serial: sh-sci: Remove unused platform data
capabilities field")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] serial: sirf: Fix out-of-bounds access through DT alias
2018-02-20 9:40 ` [PATCH 8/9] serial: sirf: " Geert Uytterhoeven
@ 2018-02-20 10:53 ` Geert Uytterhoeven
0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 10:53 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Greg Kroah-Hartman, Michal Simek,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On Tue, Feb 20, 2018 at 10:40 AM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The sirf_ports[] array is indexed using a value derived from the
> "serialN" alias in DT, which may lead to an out-of-bounds access.
>
> Fix this by adding a range check.
>
> Fixes: 66c7ab1120585d18 ("serial: sirf: Fix out-of-bounds access through DT alias")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Fixes: a6ffe8966acbb66b ("serial: sirf: use dynamic method allocate
uart structure")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 10:38 ` Geert Uytterhoeven
@ 2018-02-20 11:27 ` Michal Simek
2018-02-20 12:27 ` Geert Uytterhoeven
0 siblings, 1 reply; 24+ messages in thread
From: Michal Simek @ 2018-02-20 11:27 UTC (permalink / raw)
To: Geert Uytterhoeven, Michal Simek
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Geert Uytterhoeven, Greg Kroah-Hartman,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On 20.2.2018 11:38, Geert Uytterhoeven wrote:
> Hi Michal,
>
> On Tue, Feb 20, 2018 at 11:22 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 20.2.2018 10:40, Geert Uytterhoeven wrote:
>>> The cdns_uart_port[] array is indexed using a value derived from the
>>> "serialN" alias in DT, which may lead to an out-of-bounds access.
>>>
>>> Fix this by adding a range check.
>>>
>>> Fixes: 1f118c02a1819856 ("serial: xuartps: Fix out-of-bounds access through DT alias")
>>
>> I didn't find this sha1 - patch name is this one.
>
> Bummer, I totally screwed up my scripting...
>
> Fixes: 928e9263492069ee ("tty: xuartps: Initialize ports according to aliases")
>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> drivers/tty/serial/xilinx_uartps.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
>>> index b9b2bc76bcac606c..abcb4d09a2d866d0 100644
>>> --- a/drivers/tty/serial/xilinx_uartps.c
>>> +++ b/drivers/tty/serial/xilinx_uartps.c
>>> @@ -1110,7 +1110,7 @@ static struct uart_port *cdns_uart_get_port(int id)
>>> struct uart_port *port;
>>>
>>> /* Try the given port id if failed use default method */
>>> - if (cdns_uart_port[id].mapbase != 0) {
>>> + if (id < CDNS_UART_NR_PORTS && cdns_uart_port[id].mapbase != 0) {
>>> /* Find the next unused port */
>>> for (id = 0; id < CDNS_UART_NR_PORTS; id++)
>>> if (cdns_uart_port[id].mapbase == 0)
>>>
>>
>> Below should be better fix for this driver.
>
> I considered that, too, but...
>
>> --- a/drivers/tty/serial/xilinx_uartps.c
>> +++ b/drivers/tty/serial/xilinx_uartps.c
>> @@ -1109,6 +1109,9 @@ static struct uart_port *cdns_uart_get_port(int id)
>> {
>> struct uart_port *port;
>>
>> + if (id >= CDNS_UART_NR_PORTS)
>> + return NULL;
>> +
>> /* Try the given port id if failed use default method */
>> if (cdns_uart_port[id].mapbase != 0) {
>> /* Find the next unused port */
>> @@ -1117,9 +1120,6 @@ static struct uart_port *cdns_uart_get_port(int id)
>> break;
>> }
>>
>> - if (id >= CDNS_UART_NR_PORTS)
>> - return NULL;
>> -
>
> ... the above check cannot be removed, as it is needed to support the loop
> above to find an unused port.
You are right.
I have checked 4 patches I have sent in past which didn't reach mainline
(probably because of RFC)
Take a look at
https://www.spinics.net/lists/linux-serial/msg27106.html
I have removed cdns_uart_port array completely there.
Thanks,
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 11:27 ` Michal Simek
@ 2018-02-20 12:27 ` Geert Uytterhoeven
2018-02-20 12:39 ` Michal Simek
0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2018-02-20 12:27 UTC (permalink / raw)
To: Michal Simek
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Geert Uytterhoeven, Greg Kroah-Hartman,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
Hi Michal,
On Tue, Feb 20, 2018 at 12:27 PM, Michal Simek <michal.simek@xilinx.com> wrote:
> On 20.2.2018 11:38, Geert Uytterhoeven wrote:
>> On Tue, Feb 20, 2018 at 11:22 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>> On 20.2.2018 10:40, Geert Uytterhoeven wrote:
>>>> The cdns_uart_port[] array is indexed using a value derived from the
>>>> "serialN" alias in DT, which may lead to an out-of-bounds access.
>>>>
>>>> Fix this by adding a range check.
> I have checked 4 patches I have sent in past which didn't reach mainline
> (probably because of RFC)
> Take a look at
> https://www.spinics.net/lists/linux-serial/msg27106.html
>
> I have removed cdns_uart_port array completely there.
Nice! I'd love to get rid of fixed arrays in serial...
However, IMHO it's still worthwhile to fix the out-of-bounds access first,
as that fix can be backported to stable kernels easily.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
2018-02-20 12:27 ` Geert Uytterhoeven
@ 2018-02-20 12:39 ` Michal Simek
0 siblings, 0 replies; 24+ messages in thread
From: Michal Simek @ 2018-02-20 12:39 UTC (permalink / raw)
To: Geert Uytterhoeven, Michal Simek
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Barry Song, Geert Uytterhoeven, Greg Kroah-Hartman,
Linux Kernel Mailing List, Linux-Renesas, Vineet Gupta,
linux-serial, Jiri Slaby, arcml, linux-arm-kernel
On 20.2.2018 13:27, Geert Uytterhoeven wrote:
> Hi Michal,
>
> On Tue, Feb 20, 2018 at 12:27 PM, Michal Simek <michal.simek@xilinx.com> wrote:
>> On 20.2.2018 11:38, Geert Uytterhoeven wrote:
>>> On Tue, Feb 20, 2018 at 11:22 AM, Michal Simek <michal.simek@xilinx.com> wrote:
>>>> On 20.2.2018 10:40, Geert Uytterhoeven wrote:
>>>>> The cdns_uart_port[] array is indexed using a value derived from the
>>>>> "serialN" alias in DT, which may lead to an out-of-bounds access.
>>>>>
>>>>> Fix this by adding a range check.
>
>> I have checked 4 patches I have sent in past which didn't reach mainline
>> (probably because of RFC)
>> Take a look at
>> https://www.spinics.net/lists/linux-serial/msg27106.html
>>
>> I have removed cdns_uart_port array completely there.
>
> Nice! I'd love to get rid of fixed arrays in serial...
>
> However, IMHO it's still worthwhile to fix the out-of-bounds access first,
> as that fix can be backported to stable kernels easily.
I agree with you. Not a problem with your patch and for me it won't be
problem to rebase.
I would love to get rid of CDNS_UART_NR_PORTS but unfortunately this is
passed to core via .nr.
Thanks,
Michal
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2018-02-20 12:39 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-20 9:40 [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias Geert Uytterhoeven
2018-02-20 10:50 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 2/9] serial: fsl_lpuart: " Geert Uytterhoeven
2018-02-20 10:51 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 3/9] serial: imx: " Geert Uytterhoeven
2018-02-20 10:31 ` Uwe Kleine-König
2018-02-20 10:49 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 4/9] serial: mxs-auart: " Geert Uytterhoeven
2018-02-20 10:51 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 5/9] serial: pxa: " Geert Uytterhoeven
2018-02-20 10:52 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 6/9] serial: samsung: " Geert Uytterhoeven
2018-02-20 10:52 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 7/9] serial: sh-sci: " Geert Uytterhoeven
2018-02-20 10:53 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 8/9] serial: sirf: " Geert Uytterhoeven
2018-02-20 10:53 ` Geert Uytterhoeven
2018-02-20 9:40 ` [PATCH 9/9] serial: xuartps: " Geert Uytterhoeven
2018-02-20 10:22 ` Michal Simek
2018-02-20 10:38 ` Geert Uytterhoeven
2018-02-20 11:27 ` Michal Simek
2018-02-20 12:27 ` Geert Uytterhoeven
2018-02-20 12:39 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).