| Message ID | 1519119624-1268-4-git-send-email-geert+renesas@glider.be |
|---|---|
| State | New |
| Headers | show |
| Series | serial: Fix out-of-bounds accesses through DT aliases | expand |
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
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
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);
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(+)