[PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585..2c175b9 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.9.0

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Heiner Kallweit]

Am 24.10.2016 um 19:27 schrieb Mark Brown:
> On Tue, Oct 18, 2016 at 12:13:38AM +0200, Arnd Bergmann wrote:
>> When we get a spurious interrupt in fsl_espi_irq, we end up
>> processing four uninitalized bytes of data, as shown in this
>> warning message:
> 
> This doesn't apply against current code, please check and resend.
> 
The not yet reviewed part of my patch series from Oct 2nd,
namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
from RX FIFO" replaces the code in question.
There's more to fix like removing polling from the ISR.
If you prefer to apply Arnd's fix first I'd rebase the open part
of the patch series and resend it.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> Am 24.10.2016 um 19:27 schrieb Mark Brown:

> > This doesn't apply against current code, please check and resend.

> The not yet reviewed part of my patch series from Oct 2nd,
> namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> from RX FIFO" replaces the code in question.
> There's more to fix like removing polling from the ISR.
> If you prefer to apply Arnd's fix first I'd rebase the open part
> of the patch series and resend it.

If there are dependencies you should mention them when you resend (in
general you should always mention any unapplied or cross tree
dependencies when sending things).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

On Monday, October 24, 2016 7:45:43 PM CEST Mark Brown wrote:
> On Mon, Oct 24, 2016 at 08:36:37PM +0200, Heiner Kallweit wrote:
> > Am 24.10.2016 um 19:27 schrieb Mark Brown:
> 
> > > This doesn't apply against current code, please check and resend.
> 
> > The not yet reviewed part of my patch series from Oct 2nd,
> > namely "[PATCH 07/11] spi: fsl-espi: fix and improve reading
> > from RX FIFO" replaces the code in question.
> > There's more to fix like removing polling from the ISR.
> > If you prefer to apply Arnd's fix first I'd rebase the open part
> > of the patch series and resend it.
> 
> If there are dependencies you should mention them when you resend (in
> general you should always mention any unapplied or cross tree
> dependencies when sending things).

I think my patch (the version I sent) should ideally make it into
v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
autobuilder with the -Wmaybe-uninitialized warning added back, and
it's one of the actual bugs I found (though rather unlikely
to hit in practice).

Merging with Heiner's patches should be trivial, and I'm pretty
sure we want the patch either way. Not sure if we need a backport,
it was introduced earlier this year in commit 6319a68011b8
("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
I now found.

	Arnd

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:

> I think my patch (the version I sent) should ideally make it into
> v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> autobuilder with the -Wmaybe-uninitialized warning added back, and
> it's one of the actual bugs I found (though rather unlikely
> to hit in practice).

> Merging with Heiner's patches should be trivial, and I'm pretty
> sure we want the patch either way. Not sure if we need a backport,
> it was introduced earlier this year in commit 6319a68011b8
> ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> I now found.

Sorry but I've lost track of which patches are being talked about here.
If there's stuff for v4.9 can you send me a version that applies on
Linus' tree and I'll merge that up into what's applied for -next?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 17/28] spi: fsl-espi: avoid processing uninitalized data on error

[Arnd Bergmann]

On Tuesday, October 25, 2016 8:13:09 PM CEST Mark Brown wrote:
> 
> Not enough information to check signature validity.	Show Details
> On Mon, Oct 24, 2016 at 10:37:53PM +0200, Arnd Bergmann wrote:
> 
> > I think my patch (the version I sent) should ideally make it into
> > v4.9 as a bugfix. This was the powerpc warning I saw from Olof's
> > autobuilder with the -Wmaybe-uninitialized warning added back, and
> > it's one of the actual bugs I found (though rather unlikely
> > to hit in practice).
> 
> > Merging with Heiner's patches should be trivial, and I'm pretty
> > sure we want the patch either way. Not sure if we need a backport,
> > it was introduced earlier this year in commit 6319a68011b8
> > ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()") as
> > I now found.
> 
> Sorry but I've lost track of which patches are being talked about here.
> If there's stuff for v4.9 can you send me a version that applies on
> Linus' tree and I'll merge that up into what's applied for -next?
> 

Done.

	Arnd

Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Mark Brown]

The patch

   spi: fsl-espi: avoid processing uninitalized data on error

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 25 Oct 2016 22:57:10 +0200
Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error

When we get a spurious interrupt in fsl_espi_irq, we end up
processing four uninitalized bytes of data, as shown in this
warning message:

   drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
   drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]

This adds another check so we skip the data in this case.

Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/spi/spi-fsl-espi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 7451585a080e..2c175b9495f7 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 		mspi->len -= rx_nr_bytes;
 
-		if (mspi->rx)
+		if (rx_nr_bytes && mspi->rx)
 			mspi->get_rx(rx_data, mspi);
 	}
 
-- 
2.8.1

Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Heiner Kallweit]

Am 26.10.2016 um 12:15 schrieb Mark Brown:
> The patch
> 
>    spi: fsl-espi: avoid processing uninitalized data on error
> 
> has been applied to the spi tree at
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.  
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
>>From 5c0ba57744b1422d528f19430dd66d6803cea86f Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 25 Oct 2016 22:57:10 +0200
> Subject: [PATCH] spi: fsl-espi: avoid processing uninitalized data on error
> 
> When we get a spurious interrupt in fsl_espi_irq, we end up
> processing four uninitalized bytes of data, as shown in this
> warning message:
> 
>    drivers/spi/spi-fsl-espi.c: In function 'fsl_espi_irq':
>    drivers/spi/spi-fsl-espi.c:462:4: warning: 'rx_data' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> This adds another check so we skip the data in this case.
> 
> Fixes: 6319a68011b8 ("spi/fsl-espi: avoid infinite loops on fsl_espi_cpu_irq()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/spi/spi-fsl-espi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
> index 7451585a080e..2c175b9495f7 100644
> --- a/drivers/spi/spi-fsl-espi.c
> +++ b/drivers/spi/spi-fsl-espi.c
> @@ -458,7 +458,7 @@ static void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
>  
>  		mspi->len -= rx_nr_bytes;
>  
> -		if (mspi->rx)
> +		if (rx_nr_bytes && mspi->rx)
>  			mspi->get_rx(rx_data, mspi);
>  	}
>  
> 
There seems to be a merge problem. Before the relevant code was:
(changed in recent commit "spi: fsl-espi: fix handling of word
sizes other than 8 bit")

if (mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

Now it's:

if (rx_nr_bytes && mspi->rx) {
	mspi->get_rx(rx_data, mspi);
	mspi->rx += 4;
}

Instead it should be:

if (rx_nr_bytes && mspi->rx) {
	*(u32 *)mspi->rx = rx_data;
	mspi->rx += 4;
}

Re: Merge problem: Re: Applied "spi: fsl-espi: avoid processing uninitalized data on error" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

On Wed, Oct 26, 2016 at 08:11:28PM +0200, Heiner Kallweit wrote:

> Instead it should be:
> 
> if (rx_nr_bytes && mspi->rx) {
> 	*(u32 *)mspi->rx = rx_data;
> 	mspi->rx += 4;
> }

Please send a patch.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]