From a4ee9da1ef09f9ddb04060e644b9c34fd532c189 Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Wed, 14 Oct 2020 14:15:28 -0700 Subject: [PATCH] spi: bcm2835: Fix use-after-free in bcm2835_spi_remove() In bcm2835_spi_remove(), spi_controller_unregister() will free the ctlr reference which will lead to an use after free in bcm2835_release_dma(). To avoid this use after free, allocate the bcm2835_spi structure with a different lifecycle than the spi_controller structure such that we unregister the SPI controller, free up all the resources and finally let device managed allocations free the bcm2835_spi structure. Fixes: 05897c710e8e ("spi: bcm2835: Tear down DMA before turning off SPI controller") Fixes: 3ecd37edaa2a ("spi: bcm2835: enable dma modes for transfers meeting certain conditions") Signed-off-by: Florian Fainelli --- drivers/spi/spi-bcm2835.c | 46 +++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 41986ac0fbfb..d66358e6b5cd 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -847,42 +847,41 @@ static bool bcm2835_spi_can_dma(struct spi_controller *ctlr, return true; } -static void bcm2835_dma_release(struct spi_controller *ctlr, - struct bcm2835_spi *bs) +static void bcm2835_dma_release(struct bcm2835_spi *bs, + struct dma_chan *dma_tx, + struct dma_chan *dma_rx) { int i; - if (ctlr->dma_tx) { - dmaengine_terminate_sync(ctlr->dma_tx); + if (dma_tx) { + dmaengine_terminate_sync(dma_tx); if (bs->fill_tx_desc) dmaengine_desc_free(bs->fill_tx_desc); if (bs->fill_tx_addr) - dma_unmap_page_attrs(ctlr->dma_tx->device->dev, + dma_unmap_page_attrs(dma_tx->device->dev, bs->fill_tx_addr, sizeof(u32), DMA_TO_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); - dma_release_channel(ctlr->dma_tx); - ctlr->dma_tx = NULL; + dma_release_channel(dma_tx); } - if (ctlr->dma_rx) { - dmaengine_terminate_sync(ctlr->dma_rx); + if (dma_rx) { + dmaengine_terminate_sync(dma_rx); for (i = 0; i < BCM2835_SPI_NUM_CS; i++) if (bs->clear_rx_desc[i]) dmaengine_desc_free(bs->clear_rx_desc[i]); if (bs->clear_rx_addr) - dma_unmap_single(ctlr->dma_rx->device->dev, + dma_unmap_single(dma_rx->device->dev, bs->clear_rx_addr, sizeof(bs->clear_rx_cs), DMA_TO_DEVICE); - dma_release_channel(ctlr->dma_rx); - ctlr->dma_rx = NULL; + dma_release_channel(dma_rx); } } @@ -1010,7 +1009,7 @@ static int bcm2835_dma_init(struct spi_controller *ctlr, struct device *dev, dev_err(dev, "issue configuring dma: %d - not using DMA mode\n", ret); err_release: - bcm2835_dma_release(ctlr, bs); + bcm2835_dma_release(bs, ctlr->dma_tx, ctlr->dma_rx); err: /* * Only report error for deferred probing, otherwise fall back to @@ -1291,12 +1290,17 @@ static int bcm2835_spi_probe(struct platform_device *pdev) struct bcm2835_spi *bs; int err; + bs = devm_kzalloc(&pdev->dev, sizeof(*bs), GFP_KERNEL); + if (!bs) + return -ENOMEM; + ctlr = spi_alloc_master(&pdev->dev, ALIGN(sizeof(*bs), dma_get_cache_alignment())); if (!ctlr) return -ENOMEM; - platform_set_drvdata(pdev, ctlr); + spi_controller_set_devdata(ctlr, bs); + platform_set_drvdata(pdev, bs); ctlr->use_gpio_descriptors = true; ctlr->mode_bits = BCM2835_SPI_MODE_BITS; @@ -1308,7 +1312,6 @@ static int bcm2835_spi_probe(struct platform_device *pdev) ctlr->prepare_message = bcm2835_spi_prepare_message; ctlr->dev.of_node = pdev->dev.of_node; - bs = spi_controller_get_devdata(ctlr); bs->ctlr = ctlr; bs->regs = devm_platform_ioremap_resource(pdev, 0); @@ -1362,7 +1365,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev) return 0; out_dma_release: - bcm2835_dma_release(ctlr, bs); + bcm2835_dma_release(bs, ctlr->dma_tx, ctlr->dma_rx); out_clk_disable: clk_disable_unprepare(bs->clk); out_controller_put: @@ -1372,14 +1375,19 @@ static int bcm2835_spi_probe(struct platform_device *pdev) static int bcm2835_spi_remove(struct platform_device *pdev) { - struct spi_controller *ctlr = platform_get_drvdata(pdev); - struct bcm2835_spi *bs = spi_controller_get_devdata(ctlr); + struct bcm2835_spi *bs = platform_get_drvdata(pdev); + struct spi_controller *ctlr = bs->ctlr; + struct dma_chan *tx_chan = ctlr->dma_tx; + struct dma_chan *rx_chan = ctlr->dma_rx; bcm2835_debugfs_remove(bs); spi_unregister_controller(ctlr); - bcm2835_dma_release(ctlr, bs); + /* ctlr is freed by spi_unregister_controller() use the cached dma_chan + * references. + */ + bcm2835_dma_release(bs, tx_chan, rx_chan); /* Clear FIFOs, and disable the HW block */ bcm2835_wr(bs, BCM2835_SPI_CS, -- 2.25.1