All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
@ 2022-07-19  7:22 Marc Kleine-Budde
  2022-07-19  7:47 ` Lukas Wunner
  2022-07-20 17:45 ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-07-19  7:22 UTC (permalink / raw)
  To: linux-spi; +Cc: kernel, Marc Kleine-Budde, Lukas Wunner

In case a IRQ based transfer times out the bcm2835_spi_handle_err()
function is called. Since commit 1513ceee70f2 ("spi: bcm2835: Drop
dma_pending flag") the TX and RX DMA transfers are unconditionally
canceled, leading to NULL pointer derefs if ctlr->dma_tx or
ctlr->dma_rx are not set.

Fix the NULL pointer deref by checking that ctlr->dma_tx and
ctlr->dma_rx are valid pointers before accessing them.

Fixes: 1513ceee70f2 ("spi: bcm2835: Drop dma_pending flag")
Cc: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/spi/spi-bcm2835.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 775c0bf2f923..0933948d7df3 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -1138,10 +1138,14 @@ static void bcm2835_spi_handle_err(struct spi_controller *ctlr,
 	struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr);
 
 	/* if an error occurred and we have an active dma, then terminate */
-	dmaengine_terminate_sync(ctlr->dma_tx);
-	bs->tx_dma_active = false;
-	dmaengine_terminate_sync(ctlr->dma_rx);
-	bs->rx_dma_active = false;
+	if (ctlr->dma_tx) {
+		dmaengine_terminate_sync(ctlr->dma_tx);
+		bs->tx_dma_active = false;
+	}
+	if (ctlr->dma_rx) {
+		dmaengine_terminate_sync(ctlr->dma_rx);
+		bs->rx_dma_active = false;
+	}
 	bcm2835_spi_undo_prologue(bs);
 
 	/* and reset */
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
  2022-07-19  7:22 [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers Marc Kleine-Budde
@ 2022-07-19  7:47 ` Lukas Wunner
  2022-07-19  7:55   ` Marc Kleine-Budde
  2022-07-19 10:33   ` Mark Brown
  2022-07-20 17:45 ` Mark Brown
  1 sibling, 2 replies; 6+ messages in thread
From: Lukas Wunner @ 2022-07-19  7:47 UTC (permalink / raw)
  To: Marc Kleine-Budde, Mark Brown
  Cc: linux-spi, kernel, Stefan Wahren, Florian Fainelli, Jens Lindahl

[+cc Jens, Florian, Stefan, Mark]

On Tue, Jul 19, 2022 at 09:22:35AM +0200, Marc Kleine-Budde wrote:
> In case a IRQ based transfer times out the bcm2835_spi_handle_err()
> function is called. Since commit 1513ceee70f2 ("spi: bcm2835: Drop
> dma_pending flag") the TX and RX DMA transfers are unconditionally
> canceled, leading to NULL pointer derefs if ctlr->dma_tx or
> ctlr->dma_rx are not set.
> 
> Fix the NULL pointer deref by checking that ctlr->dma_tx and
> ctlr->dma_rx are valid pointers before accessing them.
> 
> Fixes: 1513ceee70f2 ("spi: bcm2835: Drop dma_pending flag")
> Cc: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

Link: https://lore.kernel.org/linux-spi/20220603142340.42271-1-jensctl@gmail.com/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
  2022-07-19  7:47 ` Lukas Wunner
@ 2022-07-19  7:55   ` Marc Kleine-Budde
  2022-07-20 11:48     ` Mark Brown
  2022-07-19 10:33   ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2022-07-19  7:55 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Mark Brown, linux-spi, kernel, Stefan Wahren, Florian Fainelli,
	Jens Lindahl

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

On 19.07.2022 09:47:01, Lukas Wunner wrote:
> [+cc Jens, Florian, Stefan, Mark]
> 
> On Tue, Jul 19, 2022 at 09:22:35AM +0200, Marc Kleine-Budde wrote:
> > In case a IRQ based transfer times out the bcm2835_spi_handle_err()
> > function is called. Since commit 1513ceee70f2 ("spi: bcm2835: Drop
> > dma_pending flag") the TX and RX DMA transfers are unconditionally
> > canceled, leading to NULL pointer derefs if ctlr->dma_tx or
> > ctlr->dma_rx are not set.
> > 
> > Fix the NULL pointer deref by checking that ctlr->dma_tx and
> > ctlr->dma_rx are valid pointers before accessing them.
> > 
> > Fixes: 1513ceee70f2 ("spi: bcm2835: Drop dma_pending flag")
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> Link: https://lore.kernel.org/linux-spi/20220603142340.42271-1-jensctl@gmail.com/

Thanks.

The difference is (Jens Lindahl):

+	if (bs->tx_dma_active) {
+		dmaengine_terminate_sync(ctlr->dma_tx);
+		bs->tx_dma_active = false;
+	}

vs. (me):

+	if (ctlr->dma_tx) {
+		dmaengine_terminate_sync(ctlr->dma_tx);
+		bs->tx_dma_active = false;
+	}

Which one is preferred?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
  2022-07-19  7:47 ` Lukas Wunner
  2022-07-19  7:55   ` Marc Kleine-Budde
@ 2022-07-19 10:33   ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2022-07-19 10:33 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Marc Kleine-Budde, linux-spi, kernel, Stefan Wahren,
	Florian Fainelli, Jens Lindahl

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

On Tue, Jul 19, 2022 at 09:47:01AM +0200, Lukas Wunner wrote:
> [+cc Jens, Florian, Stefan, Mark]

Can someone send me the actual patch please?

As documented in submitting-patches.rst please send patches to the 
maintainers for the code you would like to change.  The normal kernel
workflow is that people apply patches from their inboxes, if they aren't
copied they are likely to not see the patch at all and it is much more
difficult to apply patches.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
  2022-07-19  7:55   ` Marc Kleine-Budde
@ 2022-07-20 11:48     ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2022-07-20 11:48 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Lukas Wunner, linux-spi, kernel, Stefan Wahren, Florian Fainelli,
	Jens Lindahl

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

On Tue, Jul 19, 2022 at 09:55:15AM +0200, Marc Kleine-Budde wrote:

> The difference is (Jens Lindahl):
> 
> +	if (bs->tx_dma_active) {
> +		dmaengine_terminate_sync(ctlr->dma_tx);
> +		bs->tx_dma_active = false;
> +	}
> 
> vs. (me):
> 
> +	if (ctlr->dma_tx) {
> +		dmaengine_terminate_sync(ctlr->dma_tx);
> +		bs->tx_dma_active = false;
> +	}
> 
> Which one is preferred?

Probably checking dma_tx is a bit more robust, though it shouldn't
matter really.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
  2022-07-19  7:22 [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers Marc Kleine-Budde
  2022-07-19  7:47 ` Lukas Wunner
@ 2022-07-20 17:45 ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2022-07-20 17:45 UTC (permalink / raw)
  To: linux-spi, mkl; +Cc: kernel, lukas

On Tue, 19 Jul 2022 09:22:35 +0200, Marc Kleine-Budde wrote:
> In case a IRQ based transfer times out the bcm2835_spi_handle_err()
> function is called. Since commit 1513ceee70f2 ("spi: bcm2835: Drop
> dma_pending flag") the TX and RX DMA transfers are unconditionally
> canceled, leading to NULL pointer derefs if ctlr->dma_tx or
> ctlr->dma_rx are not set.
> 
> Fix the NULL pointer deref by checking that ctlr->dma_tx and
> ctlr->dma_rx are valid pointers before accessing them.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers
      commit: 4ceaa684459d414992acbefb4e4c31f2dfc50641

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-07-20 17:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19  7:22 [PATCH] spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers Marc Kleine-Budde
2022-07-19  7:47 ` Lukas Wunner
2022-07-19  7:55   ` Marc Kleine-Budde
2022-07-20 11:48     ` Mark Brown
2022-07-19 10:33   ` Mark Brown
2022-07-20 17:45 ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.