[PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Tobias Klauser]

From: Yuriy Kozlov <ykozlov@ptcusa.com>

Follow altera_jtag_uart.  This fixes a crash if there is a mistake in the DTS.

Signed-off-by: Yuriy Kozlov <ykozlov@ptcusa.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/tty/serial/altera_uart.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index e790375..1f03309 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -556,7 +556,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
 	res_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res_mem)
 		port->mapbase = res_mem->start;
-	else if (platp->mapbase)
+	else if (platp)
 		port->mapbase = platp->mapbase;
 	else
 		return -EINVAL;
@@ -564,7 +564,7 @@ static int __devinit altera_uart_probe(struct platform_device *pdev)
 	res_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (res_irq)
 		port->irq = res_irq->start;
-	else if (platp->irq)
+	else if (platp)
 		port->irq = platp->irq;
 
 	/* Check platform data first so we can override device node data */
-- 
1.7.5.4

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Greg Kroah-Hartman]

On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> From: Yuriy Kozlov <ykozlov@ptcusa.com>
> 
> Follow altera_jtag_uart.  This fixes a crash if there is a mistake in the DTS.

Is this something you have seen in the wild?  Is this something to be
backported to stable kernels?

thanks,

greg k-h

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Tobias Klauser]

On 2012-03-29 at 18:13:37 +0200, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> > From: Yuriy Kozlov <ykozlov@ptcusa.com>
> > 
> > Follow altera_jtag_uart.  This fixes a crash if there is a mistake in the DTS.
> 
> Is this something you have seen in the wild?  Is this something to be
> backported to stable kernels?

Not that I know of. But nevertheless it might be worthwhile. Shall I
send the patch again with Cc: stable@kernel.org?

Thanks
Tobias

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Greg Kroah-Hartman]

On Thu, Mar 29, 2012 at 09:38:55PM +0200, Tobias Klauser wrote:
> On 2012-03-29 at 18:13:37 +0200, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> > > From: Yuriy Kozlov <ykozlov@ptcusa.com>
> > > 
> > > Follow altera_jtag_uart.  This fixes a crash if there is a mistake in the DTS.
> > 
> > Is this something you have seen in the wild?  Is this something to be
> > backported to stable kernels?
> 
> Not that I know of. But nevertheless it might be worthwhile. Shall I
> send the patch again with Cc: stable@kernel.org?

Nope, I can add it.

But, if this isn't something that you have seen, why is it needed to be
checked for?  What is this making work properly that isn't working
properly now?

thanks,

greg k-h

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Yuriy Kozlov]

On Thursday, March 29, 2012 12:47:11 Greg Kroah-Hartman wrote:
> On Thu, Mar 29, 2012 at 09:38:55PM +0200, Tobias Klauser wrote:
> > On 2012-03-29 at 18:13:37 +0200, Greg Kroah-Hartman 
<gregkh@linuxfoundation.org> wrote:
> > > On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> > > > From: Yuriy Kozlov <ykozlov@ptcusa.com>
> > > > 
> > > > Follow altera_jtag_uart.  This fixes a crash if there is a mistake in
> > > > the DTS.> > 
> > > Is this something you have seen in the wild?  Is this something to be
> > > backported to stable kernels?
> > 
> > Not that I know of. But nevertheless it might be worthwhile. Shall I
> > send the patch again with Cc: stable@kernel.org?
> 
> Nope, I can add it.
> 
> But, if this isn't something that you have seen, why is it needed to be
> checked for?  What is this making work properly that isn't working
> properly now?
> 
> thanks,
> 
> greg k-h

The driver is designed to use Device Tree to get hardware information, so 
platp (platform_data) is usually NULL.  platform_data is still used as a 
fallback mechanism.  The problem arises when the device is in the DT, but is 
missing a required property (address or irq).  The driver falls back to 
looking for it in the null platp and panics.  The function actually already 
checks platp for other properties later on, but under this condition it tried 
to dereference it before the null checks.

-- 
~ Yuriy Kozlov
 Pyramid Technical Consultants, Inc.
---------------------------------------------------------

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Greg Kroah-Hartman]

On Thu, Mar 29, 2012 at 04:02:07PM -0400, Yuriy Kozlov wrote:
> On Thursday, March 29, 2012 12:47:11 Greg Kroah-Hartman wrote:
> > On Thu, Mar 29, 2012 at 09:38:55PM +0200, Tobias Klauser wrote:
> > > On 2012-03-29 at 18:13:37 +0200, Greg Kroah-Hartman 
> <gregkh@linuxfoundation.org> wrote:
> > > > On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> > > > > From: Yuriy Kozlov <ykozlov@ptcusa.com>
> > > > > 
> > > > > Follow altera_jtag_uart.  This fixes a crash if there is a mistake in
> > > > > the DTS.> > 
> > > > Is this something you have seen in the wild?  Is this something to be
> > > > backported to stable kernels?
> > > 
> > > Not that I know of. But nevertheless it might be worthwhile. Shall I
> > > send the patch again with Cc: stable@kernel.org?
> > 
> > Nope, I can add it.
> > 
> > But, if this isn't something that you have seen, why is it needed to be
> > checked for?  What is this making work properly that isn't working
> > properly now?
> > 
> > thanks,
> > 
> > greg k-h
> 
> The driver is designed to use Device Tree to get hardware information, so 
> platp (platform_data) is usually NULL.  platform_data is still used as a 
> fallback mechanism.  The problem arises when the device is in the DT, but is 
> missing a required property (address or irq).  The driver falls back to 
> looking for it in the null platp and panics.  The function actually already 
> checks platp for other properties later on, but under this condition it tried 
> to dereference it before the null checks.

And again, does this happen today in the 3.4-rc1 kernel?  3.3?  3.2?
3.0?  Something else?

If not, then why make this change?  If so, it needs to go to older
kernels, right?

thanks,

greg k-h

Re: [PATCH] tty: serial: altera_uart: Check for NULL platform_data in probe.

[Tobias Klauser]

On 2012-03-29 at 23:39:57 +0200, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Thu, Mar 29, 2012 at 04:02:07PM -0400, Yuriy Kozlov wrote:
> > On Thursday, March 29, 2012 12:47:11 Greg Kroah-Hartman wrote:
> > > On Thu, Mar 29, 2012 at 09:38:55PM +0200, Tobias Klauser wrote:
> > > > On 2012-03-29 at 18:13:37 +0200, Greg Kroah-Hartman 
> > <gregkh@linuxfoundation.org> wrote:
> > > > > On Thu, Mar 29, 2012 at 09:55:27AM +0200, Tobias Klauser wrote:
> > > > > > From: Yuriy Kozlov <ykozlov@ptcusa.com>
> > > > > > 
> > > > > > Follow altera_jtag_uart.  This fixes a crash if there is a mistake in
> > > > > > the DTS.> > 
> > > > > Is this something you have seen in the wild?  Is this something to be
> > > > > backported to stable kernels?
> > > > 
> > > > Not that I know of. But nevertheless it might be worthwhile. Shall I
> > > > send the patch again with Cc: stable@kernel.org?
> > > 
> > > Nope, I can add it.
> > > 
> > > But, if this isn't something that you have seen, why is it needed to be
> > > checked for?  What is this making work properly that isn't working
> > > properly now?
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > The driver is designed to use Device Tree to get hardware information, so 
> > platp (platform_data) is usually NULL.  platform_data is still used as a 
> > fallback mechanism.  The problem arises when the device is in the DT, but is 
> > missing a required property (address or irq).  The driver falls back to 
> > looking for it in the null platp and panics.  The function actually already 
> > checks platp for other properties later on, but under this condition it tried 
> > to dereference it before the null checks.
> 
> And again, does this happen today in the 3.4-rc1 kernel?  3.3?  3.2?
> 3.0?  Something else?

It's present in all of them, since 2.6.37 when devicetree support for
the driver was added. So it could potentially be triggered in all of
them.

> If not, then why make this change?  If so, it needs to go to older
> kernels, right?

If Yuriy has seen this in the wild, I'd suggest it should got in then.
Please let me know if I should redo the patch against the older version.

Thanks a lot
Tobias