Linux Serial subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH V2] tty: serial: imx: allow breaks to be received when using dma
From: Martin Hicks @ 2018-02-20 18:01 UTC (permalink / raw)
  To: Troy Kisky
  Cc: gregkh, mort, linux-serial, u.kleine-koenig, fabio.estevam,
	linux-arm-kernel, l.stach
In-Reply-To: <20180209212240.14095-1-troy.kisky@boundarydevices.com>

On Fri, Feb 09, 2018 at 01:22:40PM -0800, Troy Kisky wrote:
> This allows me to login after sending a break when service
> serial-getty@ttymxc0.service is running
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

Tested-by:  Martin Hicks <mort@bork.org>

Works for me.
mh

> 
> ---
> v2: rebase only
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 1d7ca382bc12..2eb8c4a20d68 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -927,7 +927,6 @@ static void dma_rx_callback(void *data)
>  	status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
>  
>  	if (status == DMA_ERROR) {
> -		dev_err(sport->port.dev, "DMA transaction error.\n");
>  		clear_rx_errors(sport);
>  		return;
>  	}
> @@ -1028,6 +1027,7 @@ static int start_rx_dma(struct imx_port *sport)
>  
>  static void clear_rx_errors(struct imx_port *sport)
>  {
> +	struct tty_port *port = &sport->port.state->port;
>  	unsigned int status_usr1, status_usr2;
>  
>  	status_usr1 = readl(sport->port.membase + USR1);
> @@ -1036,12 +1036,18 @@ static void clear_rx_errors(struct imx_port *sport)
>  	if (status_usr2 & USR2_BRCD) {
>  		sport->port.icount.brk++;
>  		writel(USR2_BRCD, sport->port.membase + USR2);
> -	} else if (status_usr1 & USR1_FRAMERR) {
> -		sport->port.icount.frame++;
> -		writel(USR1_FRAMERR, sport->port.membase + USR1);
> -	} else if (status_usr1 & USR1_PARITYERR) {
> -		sport->port.icount.parity++;
> -		writel(USR1_PARITYERR, sport->port.membase + USR1);
> +		if (tty_insert_flip_char(port, 0, TTY_BREAK) == 0)
> +			sport->port.icount.buf_overrun++;
> +		tty_flip_buffer_push(port);
> +	} else {
> +		dev_err(sport->port.dev, "DMA transaction error.\n");
> +		if (status_usr1 & USR1_FRAMERR) {
> +			sport->port.icount.frame++;
> +			writel(USR1_FRAMERR, sport->port.membase + USR1);
> +		} else if (status_usr1 & USR1_PARITYERR) {
> +			sport->port.icount.parity++;
> +			writel(USR1_PARITYERR, sport->port.membase + USR1);
> +		}
>  	}
>  
>  	if (status_usr2 & USR2_ORE) {
> -- 
> 2.14.1
> 

-- 
Martin Hicks P.Eng.    |      mort@bork.org
Bork Consulting Inc.   |  +1 (613) 266-2296

^ permalink raw reply

* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <CAMuHMdWe9XkoDKkyfu1pOG0oS0_Tdx1w=OOk05EvTr_dK=tp7Q@mail.gmail.com>

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

* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <6cabf7c6-302e-efa9-8f4d-25fd35ecddad@xilinx.com>

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

* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <CAMuHMdUOKMM5mSrnnrxCi0gX_TbjYkj7vwn11b2D5p7XigM+Ag@mail.gmail.com>

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

* Re: [PATCH 8/9] serial: sirf: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-9-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 7/9] serial: sh-sci: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-8-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 6/9] serial: samsung: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-7-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 5/9] serial: pxa: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-6-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 4/9] serial: mxs-auart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-5-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 2/9] serial: fsl_lpuart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-3-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-2-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
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
In-Reply-To: <20180220103134.xfh2zl2vgonb2ywb@pengutronix.de>

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

* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <c16b025a-a302-8515-2697-f7eca478e939@xilinx.com>

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

* Re: [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-4-git-send-email-geert+renesas@glider.be>

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

* Re: [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-10-git-send-email-geert+renesas@glider.be>

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

* [PATCH 9/9] serial: xuartps: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 8/9] serial: sirf: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 7/9] serial: sh-sci: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 6/9] serial: samsung: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 5/9] serial: pxa: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 4/9] serial: mxs-auart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 3/9] serial: imx: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 2/9] serial: fsl_lpuart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 1/9] serial: arc_uart: Fix out-of-bounds access through DT alias
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
In-Reply-To: <1519119624-1268-1-git-send-email-geert+renesas@glider.be>

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

* [PATCH 0/9] serial: Fix out-of-bounds accesses through DT aliases
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox