lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <877exxcu9q.wl%kuninori.morimoto.gx@renesas.com>
Date:   Mon, 21 Aug 2017 06:31:57 +0000
From:   Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To:     Vinod Koul <vinod.koul@...el.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Niklas Söderlund 
        <niklas.soderlund+renesas@...natech.se>,
        Dan Williams <dan.j.williams@...el.com>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Linux-Kernel <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Anton Volkov <avolkov@...ras.ru>,
        Alexey Khoroshilov <khoroshilov@...ras.ru>,
        ldv-project@...uxtesting.org, dmaengine@...r.kernel.org,
        geert+renesas@...der.be
Subject: [PATCH v2] rcar-dmac: initialize all data before registering IRQ handler


From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>

Anton Volkov noticed that engine->dev is NULL before
of_dma_controller_register() in probe.
Thus there might be a NULL pointer dereference in
rcar_dmac_chan_start_xfer while accessing chan->chan.device->dev which
is equal to (&dmac->engine)->dev.
On same reason, same and similar things will happen if we didn't
initialize all necessary data before calling register irq function.
To be more safety code, this patch initialize all necessary data
before calling register irq function.

Reported-by: Anton Volkov <avolkov@...ras.ru>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
---
v1 -> v2

 - care devm_request_threaded_irq(xxx) on rcar_dmac_chan_probe()
 - care devm_request_irq(xx) on rcar_dmac_probe()

 drivers/dma/sh/rcar-dmac.c | 85 +++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 42 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index ffcadca..2b2c7db 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1690,6 +1690,15 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
 	if (!irqname)
 		return -ENOMEM;
 
+	/*
+	 * Initialize the DMA engine channel and add it to the DMA engine
+	 * channels list.
+	 */
+	chan->device = &dmac->engine;
+	dma_cookie_init(chan);
+
+	list_add_tail(&chan->device_node, &dmac->engine.channels);
+
 	ret = devm_request_threaded_irq(dmac->dev, rchan->irq,
 					rcar_dmac_isr_channel,
 					rcar_dmac_isr_channel_thread, 0,
@@ -1700,15 +1709,6 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
 		return ret;
 	}
 
-	/*
-	 * Initialize the DMA engine channel and add it to the DMA engine
-	 * channels list.
-	 */
-	chan->device = &dmac->engine;
-	dma_cookie_init(chan);
-
-	list_add_tail(&chan->device_node, &dmac->engine.channels);
-
 	return 0;
 }
 
@@ -1794,14 +1794,6 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 	if (!irqname)
 		return -ENOMEM;
 
-	ret = devm_request_irq(&pdev->dev, irq, rcar_dmac_isr_error, 0,
-			       irqname, dmac);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n",
-			irq, ret);
-		return ret;
-	}
-
 	/* Enable runtime PM and initialize the device. */
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_get_sync(&pdev->dev);
@@ -1818,8 +1810,32 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 		goto error;
 	}
 
-	/* Initialize the channels. */
-	INIT_LIST_HEAD(&dmac->engine.channels);
+	/* Initialize engine */
+	engine = &dmac->engine;
+
+	dma_cap_set(DMA_MEMCPY, engine->cap_mask);
+	dma_cap_set(DMA_SLAVE, engine->cap_mask);
+
+	engine->dev		= &pdev->dev;
+	engine->copy_align	= ilog2(RCAR_DMAC_MEMCPY_XFER_SIZE);
+
+	engine->src_addr_widths	= widths;
+	engine->dst_addr_widths	= widths;
+	engine->directions	= BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM);
+	engine->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
+
+	engine->device_alloc_chan_resources	= rcar_dmac_alloc_chan_resources;
+	engine->device_free_chan_resources	= rcar_dmac_free_chan_resources;
+	engine->device_prep_dma_memcpy		= rcar_dmac_prep_dma_memcpy;
+	engine->device_prep_slave_sg		= rcar_dmac_prep_slave_sg;
+	engine->device_prep_dma_cyclic		= rcar_dmac_prep_dma_cyclic;
+	engine->device_config			= rcar_dmac_device_config;
+	engine->device_terminate_all		= rcar_dmac_chan_terminate_all;
+	engine->device_tx_status		= rcar_dmac_tx_status;
+	engine->device_issue_pending		= rcar_dmac_issue_pending;
+	engine->device_synchronize		= rcar_dmac_device_synchronize;
+
+	INIT_LIST_HEAD(&engine->channels);
 
 	for (i = 0; i < dmac->n_channels; ++i) {
 		ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i],
@@ -1828,6 +1844,14 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 			goto error;
 	}
 
+	ret = devm_request_irq(&pdev->dev, irq, rcar_dmac_isr_error, 0,
+			       irqname, dmac);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n",
+			irq, ret);
+		return ret;
+	}
+
 	/* Register the DMAC as a DMA provider for DT. */
 	ret = of_dma_controller_register(pdev->dev.of_node, rcar_dmac_of_xlate,
 					 NULL);
@@ -1839,29 +1863,6 @@ static int rcar_dmac_probe(struct platform_device *pdev)
 	 *
 	 * Default transfer size of 32 bytes requires 32-byte alignment.
 	 */
-	engine = &dmac->engine;
-	dma_cap_set(DMA_MEMCPY, engine->cap_mask);
-	dma_cap_set(DMA_SLAVE, engine->cap_mask);
-
-	engine->dev = &pdev->dev;
-	engine->copy_align = ilog2(RCAR_DMAC_MEMCPY_XFER_SIZE);
-
-	engine->src_addr_widths = widths;
-	engine->dst_addr_widths = widths;
-	engine->directions = BIT(DMA_MEM_TO_DEV) | BIT(DMA_DEV_TO_MEM);
-	engine->residue_granularity = DMA_RESIDUE_GRANULARITY_BURST;
-
-	engine->device_alloc_chan_resources = rcar_dmac_alloc_chan_resources;
-	engine->device_free_chan_resources = rcar_dmac_free_chan_resources;
-	engine->device_prep_dma_memcpy = rcar_dmac_prep_dma_memcpy;
-	engine->device_prep_slave_sg = rcar_dmac_prep_slave_sg;
-	engine->device_prep_dma_cyclic = rcar_dmac_prep_dma_cyclic;
-	engine->device_config = rcar_dmac_device_config;
-	engine->device_terminate_all = rcar_dmac_chan_terminate_all;
-	engine->device_tx_status = rcar_dmac_tx_status;
-	engine->device_issue_pending = rcar_dmac_issue_pending;
-	engine->device_synchronize = rcar_dmac_device_synchronize;
-
 	ret = dma_async_device_register(engine);
 	if (ret < 0)
 		goto error;
-- 
1.9.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ