| Message ID | 20220719072234.2782764-1-mkl@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | spi: bcm2835: bcm2835_spi_handle_err(): fix NULL pointer deref for non DMA transfers | expand |
[+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/
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
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.
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.
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
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 */
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(-)