dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
@ 2019-06-24 14:07 Sven Van Asbroeck
  2019-06-25  7:14 ` Robin Gong
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Sven Van Asbroeck @ 2019-06-24 14:07 UTC (permalink / raw)
  To: Robin Gong
  Cc: Vinod Koul, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine, linux-arm-kernel, linux-kernel

If probe() fails anywhere beyond the point where
sdma_get_firmware() is called, then a kernel oops may occur.

Problematic sequence of events:
1. probe() calls sdma_get_firmware(), which schedules the
   firmware callback to run when firmware becomes available,
   using the sdma instance structure as the context
2. probe() encounters an error, which deallocates the
   sdma instance structure
3. firmware becomes available, firmware callback is
   called with deallocated sdma instance structure
4. use after free - kernel oops !

Solution: only attempt to load firmware when we're certain
that probe() will succeed. This guarantees that the firmware
callback's context will remain valid.

Note that the remove() path is unaffected by this issue: the
firmware loader will increment the driver module's use count,
ensuring that the module cannot be unloaded while the
firmware callback is pending or running.

To: Robin Gong <yibin.gong@nxp.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 drivers/dma/imx-sdma.c | 48 ++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 99d9f431ae2c..3f0f41d16e1c 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2096,27 +2096,6 @@ static int sdma_probe(struct platform_device *pdev)
 	if (pdata && pdata->script_addrs)
 		sdma_add_scripts(sdma, pdata->script_addrs);
 
-	if (pdata) {
-		ret = sdma_get_firmware(sdma, pdata->fw_name);
-		if (ret)
-			dev_warn(&pdev->dev, "failed to get firmware from platform data\n");
-	} else {
-		/*
-		 * Because that device tree does not encode ROM script address,
-		 * the RAM script in firmware is mandatory for device tree
-		 * probe, otherwise it fails.
-		 */
-		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
-					      &fw_name);
-		if (ret)
-			dev_warn(&pdev->dev, "failed to get firmware name\n");
-		else {
-			ret = sdma_get_firmware(sdma, fw_name);
-			if (ret)
-				dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
-		}
-	}
-
 	sdma->dma_device.dev = &pdev->dev;
 
 	sdma->dma_device.device_alloc_chan_resources = sdma_alloc_chan_resources;
@@ -2161,6 +2140,33 @@ static int sdma_probe(struct platform_device *pdev)
 		of_node_put(spba_bus);
 	}
 
+	/*
+	 * Kick off firmware loading as the very last step:
+	 * attempt to load firmware only if we're not on the error path, because
+	 * the firmware callback requires a fully functional and allocated sdma
+	 * instance.
+	 */
+	if (pdata) {
+		ret = sdma_get_firmware(sdma, pdata->fw_name);
+		if (ret)
+			dev_warn(&pdev->dev, "failed to get firmware from platform data\n");
+	} else {
+		/*
+		 * Because that device tree does not encode ROM script address,
+		 * the RAM script in firmware is mandatory for device tree
+		 * probe, otherwise it fails.
+		 */
+		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
+					      &fw_name);
+		if (ret)
+			dev_warn(&pdev->dev, "failed to get firmware name\n");
+		else {
+			ret = sdma_get_firmware(sdma, fw_name);
+			if (ret)
+				dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
+		}
+	}
+
 	return 0;
 
 err_register:
-- 
2.17.1


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

* RE: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-06-24 14:07 [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path Sven Van Asbroeck
@ 2019-06-25  7:14 ` Robin Gong
  2019-07-05  7:28 ` Vinod Koul
  2019-07-05 12:46 ` Vinod Koul
  2 siblings, 0 replies; 9+ messages in thread
From: Robin Gong @ 2019-06-25  7:14 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Vinod Koul, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, dl-linux-imx, dmaengine,
	linux-arm-kernel, linux-kernel

Reviewed-by: Robin Gong <yibin.gong@nxp.com>
> -----Original Message-----
> From: Sven Van Asbroeck <thesven73@gmail.com>
> Subject: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
> 
> If probe() fails anywhere beyond the point where
> sdma_get_firmware() is called, then a kernel oops may occur.
> 
> Problematic sequence of events:
> 1. probe() calls sdma_get_firmware(), which schedules the
>    firmware callback to run when firmware becomes available,
>    using the sdma instance structure as the context 2. probe() encounters an
> error, which deallocates the
>    sdma instance structure
> 3. firmware becomes available, firmware callback is
>    called with deallocated sdma instance structure 4. use after free - kernel
> oops !
> 
> Solution: only attempt to load firmware when we're certain that probe() will
> succeed. This guarantees that the firmware callback's context will remain valid.
> 
> Note that the remove() path is unaffected by this issue: the firmware loader will
> increment the driver module's use count, ensuring that the module cannot be
> unloaded while the firmware callback is pending or running.
> 
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
>  drivers/dma/imx-sdma.c | 48 ++++++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> 99d9f431ae2c..3f0f41d16e1c 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -2096,27 +2096,6 @@ static int sdma_probe(struct platform_device
> *pdev)
>  	if (pdata && pdata->script_addrs)
>  		sdma_add_scripts(sdma, pdata->script_addrs);
> 
> -	if (pdata) {
> -		ret = sdma_get_firmware(sdma, pdata->fw_name);
> -		if (ret)
> -			dev_warn(&pdev->dev, "failed to get firmware from platform
> data\n");
> -	} else {
> -		/*
> -		 * Because that device tree does not encode ROM script address,
> -		 * the RAM script in firmware is mandatory for device tree
> -		 * probe, otherwise it fails.
> -		 */
> -		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
> -					      &fw_name);
> -		if (ret)
> -			dev_warn(&pdev->dev, "failed to get firmware name\n");
> -		else {
> -			ret = sdma_get_firmware(sdma, fw_name);
> -			if (ret)
> -				dev_warn(&pdev->dev, "failed to get firmware from device
> tree\n");
> -		}
> -	}
> -
>  	sdma->dma_device.dev = &pdev->dev;
> 
>  	sdma->dma_device.device_alloc_chan_resources =
> sdma_alloc_chan_resources; @@ -2161,6 +2140,33 @@ static int
> sdma_probe(struct platform_device *pdev)
>  		of_node_put(spba_bus);
>  	}
> 
> +	/*
> +	 * Kick off firmware loading as the very last step:
> +	 * attempt to load firmware only if we're not on the error path, because
> +	 * the firmware callback requires a fully functional and allocated sdma
> +	 * instance.
> +	 */
> +	if (pdata) {
> +		ret = sdma_get_firmware(sdma, pdata->fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware from platform
> data\n");
> +	} else {
> +		/*
> +		 * Because that device tree does not encode ROM script address,
> +		 * the RAM script in firmware is mandatory for device tree
> +		 * probe, otherwise it fails.
> +		 */
> +		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
> +					      &fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware name\n");
> +		else {
> +			ret = sdma_get_firmware(sdma, fw_name);
> +			if (ret)
> +				dev_warn(&pdev->dev, "failed to get firmware from device
> tree\n");
> +		}
> +	}
> +
>  	return 0;
> 
>  err_register:
> --
> 2.17.1


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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-06-24 14:07 [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path Sven Van Asbroeck
  2019-06-25  7:14 ` Robin Gong
@ 2019-07-05  7:28 ` Vinod Koul
  2019-07-05 12:26   ` Sven Van Asbroeck
  2019-07-05 12:46 ` Vinod Koul
  2 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2019-07-05  7:28 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine, linux-arm-kernel, linux-kernel

On 24-06-19, 10:07, Sven Van Asbroeck wrote:
> If probe() fails anywhere beyond the point where
> sdma_get_firmware() is called, then a kernel oops may occur.
> 
> Problematic sequence of events:
> 1. probe() calls sdma_get_firmware(), which schedules the
>    firmware callback to run when firmware becomes available,
>    using the sdma instance structure as the context
> 2. probe() encounters an error, which deallocates the
>    sdma instance structure
> 3. firmware becomes available, firmware callback is
>    called with deallocated sdma instance structure
> 4. use after free - kernel oops !
> 
> Solution: only attempt to load firmware when we're certain
> that probe() will succeed. This guarantees that the firmware
> callback's context will remain valid.
> 
> Note that the remove() path is unaffected by this issue: the
> firmware loader will increment the driver module's use count,
> ensuring that the module cannot be unloaded while the
> firmware callback is pending or running.
> 
> To: Robin Gong <yibin.gong@nxp.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
>  drivers/dma/imx-sdma.c | 48 ++++++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 99d9f431ae2c..3f0f41d16e1c 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -2096,27 +2096,6 @@ static int sdma_probe(struct platform_device *pdev)
>  	if (pdata && pdata->script_addrs)
>  		sdma_add_scripts(sdma, pdata->script_addrs);
>  
> -	if (pdata) {
> -		ret = sdma_get_firmware(sdma, pdata->fw_name);
> -		if (ret)
> -			dev_warn(&pdev->dev, "failed to get firmware from platform data\n");
> -	} else {
> -		/*
> -		 * Because that device tree does not encode ROM script address,
> -		 * the RAM script in firmware is mandatory for device tree
> -		 * probe, otherwise it fails.
> -		 */
> -		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
> -					      &fw_name);
> -		if (ret)
> -			dev_warn(&pdev->dev, "failed to get firmware name\n");
> -		else {
> -			ret = sdma_get_firmware(sdma, fw_name);
> -			if (ret)
> -				dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
> -		}
> -	}
> -
>  	sdma->dma_device.dev = &pdev->dev;
>  
>  	sdma->dma_device.device_alloc_chan_resources = sdma_alloc_chan_resources;
> @@ -2161,6 +2140,33 @@ static int sdma_probe(struct platform_device *pdev)
>  		of_node_put(spba_bus);
>  	}
>  
> +	/*
> +	 * Kick off firmware loading as the very last step:
> +	 * attempt to load firmware only if we're not on the error path, because
> +	 * the firmware callback requires a fully functional and allocated sdma
> +	 * instance.
> +	 */
> +	if (pdata) {
> +		ret = sdma_get_firmware(sdma, pdata->fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware from platform data\n");
> +	} else {
> +		/*
> +		 * Because that device tree does not encode ROM script address,
> +		 * the RAM script in firmware is mandatory for device tree
> +		 * probe, otherwise it fails.
> +		 */
> +		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
> +					      &fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware name\n");

if should have braces!

> +		else {
> +			ret = sdma_get_firmware(sdma, fw_name);
> +			if (ret)
> +				dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
> +		}
> +	}
> +
>  	return 0;
>  
>  err_register:
> -- 
> 2.17.1

Applied after fixing braces!


-- 
~Vinod

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-07-05  7:28 ` Vinod Koul
@ 2019-07-05 12:26   ` Sven Van Asbroeck
  2019-07-05 15:47     ` Lothar Waßmann
  0 siblings, 1 reply; 9+ messages in thread
From: Sven Van Asbroeck @ 2019-07-05 12:26 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Kernel Mailing List

Hi Vinod,

On Fri, Jul 5, 2019 at 3:32 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> > +             if (ret)
> > +                     dev_warn(&pdev->dev, "failed to get firmware name\n");
>
> if should have braces!
> Applied after fixing braces!

checkpatch.pl output after adding braces:

WARNING: braces {} are not necessary for single statement blocks
#102: FILE: drivers/dma/imx-sdma.c:2165:
+ if (ret) {
+ dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
+ }

total: 0 errors, 1 warnings, 61 lines checked

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-06-24 14:07 [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path Sven Van Asbroeck
  2019-06-25  7:14 ` Robin Gong
  2019-07-05  7:28 ` Vinod Koul
@ 2019-07-05 12:46 ` Vinod Koul
  2019-07-05 13:08   ` Sven Van Asbroeck
  2 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2019-07-05 12:46 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine, linux-arm-kernel, linux-kernel

On 24-06-19, 10:07, Sven Van Asbroeck wrote:

Quoting orignal patch for better context:

> +	if (pdata) {
> +		ret = sdma_get_firmware(sdma, pdata->fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware from platform data\n");
> +	} else {
> +		/*
> +		 * Because that device tree does not encode ROM script address,
> +		 * the RAM script in firmware is mandatory for device tree
> +		 * probe, otherwise it fails.
> +		 */
> +		ret = of_property_read_string(np, "fsl,sdma-ram-script-name",
> +					      &fw_name);
> +		if (ret)
> +			dev_warn(&pdev->dev, "failed to get firmware name\n");
> +		else {
> +			ret = sdma_get_firmware(sdma, fw_name);
> +			if (ret)
> +				dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
> +		}
> +	}
> +

On 05-07-19, 08:26, Sven Van Asbroeck wrote:
> Hi Vinod,
> 
> On Fri, Jul 5, 2019 at 3:32 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > > +             if (ret)
> > > +                     dev_warn(&pdev->dev, "failed to get firmware name\n");
> >
> > if should have braces!
> > Applied after fixing braces!
> 
> checkpatch.pl output after adding braces:
> 
> WARNING: braces {} are not necessary for single statement blocks
> #102: FILE: drivers/dma/imx-sdma.c:2165:
> + if (ret) {
> + dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
> + }

there is an else here too!

With the patch the checkpatch warns:

CHECK: braces {} should be used on all arms of this statement
#201: FILE: drivers/dma/imx-sdma.c:2161:
+		if (ret)
[...]
+		else {
[...]


Even if the if is a single block and else being multi block, if would
need matching brances. With the change pushed:

total: 0 errors, 0 warnings, 0 checks, 60 lines checked


-- 
~Vinod

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-07-05 12:46 ` Vinod Koul
@ 2019-07-05 13:08   ` Sven Van Asbroeck
  2019-07-05 13:10     ` Fabio Estevam
  2019-07-05 13:10     ` Vinod Koul
  0 siblings, 2 replies; 9+ messages in thread
From: Sven Van Asbroeck @ 2019-07-05 13:08 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Kernel Mailing List

Hi Vinod,

On Fri, Jul 5, 2019 at 8:50 AM Vinod Koul <vkoul@kernel.org> wrote:
>
> there is an else here too!
>

You are of course right, I was looking at the wrong if.

Apologies for the confusion, I did try to look at what you
changed, but your git repo listed in MAINTAINERS appears
unresponsive for me?

Thanks, Sven

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-07-05 13:08   ` Sven Van Asbroeck
@ 2019-07-05 13:10     ` Fabio Estevam
  2019-07-05 13:10     ` Vinod Koul
  1 sibling, 0 replies; 9+ messages in thread
From: Fabio Estevam @ 2019-07-05 13:10 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Vinod Koul, Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, NXP Linux Team, dmaengine,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Kernel Mailing List

On Fri, Jul 5, 2019 at 10:09 AM Sven Van Asbroeck <thesven73@gmail.com> wrote:

> You are of course right, I was looking at the wrong if.
>
> Apologies for the confusion, I did try to look at what you
> changed, but your git repo listed in MAINTAINERS appears
> unresponsive for me?

Works fine here:
https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git/commit/?h=fixes&id=2b8066c3deb9140fdf258417a51479b2aeaa7622

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-07-05 13:08   ` Sven Van Asbroeck
  2019-07-05 13:10     ` Fabio Estevam
@ 2019-07-05 13:10     ` Vinod Koul
  1 sibling, 0 replies; 9+ messages in thread
From: Vinod Koul @ 2019-07-05 13:10 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Robin Gong, Dan Williams, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dmaengine,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Linux Kernel Mailing List

On 05-07-19, 09:08, Sven Van Asbroeck wrote:
> Hi Vinod,
> 
> On Fri, Jul 5, 2019 at 8:50 AM Vinod Koul <vkoul@kernel.org> wrote:
> >
> > there is an else here too!
> >
> 
> You are of course right, I was looking at the wrong if.

NO issues :)
> 
> Apologies for the confusion, I did try to look at what you
> changed, but your git repo listed in MAINTAINERS appears
> unresponsive for me?

To quote David you need to move to 21st century (like me). IPv4 switch
for infradead is down so...

meanwhile this would work too:
git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git

-- 
~Vinod

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

* Re: [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path
  2019-07-05 12:26   ` Sven Van Asbroeck
@ 2019-07-05 15:47     ` Lothar Waßmann
  0 siblings, 0 replies; 9+ messages in thread
From: Lothar Waßmann @ 2019-07-05 15:47 UTC (permalink / raw)
  To: Sven Van Asbroeck
  Cc: Vinod Koul, Shawn Guo, Sascha Hauer, Linux Kernel Mailing List,
	NXP Linux Team, Pengutronix Kernel Team, dmaengine, Dan Williams,
	Robin Gong, Fabio Estevam,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

Hi,

On Fri, 5 Jul 2019 08:26:12 -0400 Sven Van Asbroeck wrote:
> Hi Vinod,
> 
> On Fri, Jul 5, 2019 at 3:32 AM Vinod Koul <vkoul@kernel.org> wrote:
> >  
> > > +             if (ret)
> > > +                     dev_warn(&pdev->dev, "failed to get firmware name\n");  
> >
> > if should have braces!
> > Applied after fixing braces!  
> 
> checkpatch.pl output after adding braces:
> 
> WARNING: braces {} are not necessary for single statement blocks
> #102: FILE: drivers/dma/imx-sdma.c:2165:
> + if (ret) {
> + dev_warn(&pdev->dev, "failed to get firmware from device tree\n");
> + }
> 
You changed the braces in the wrong place!
The comment applied to the previous 'if (ret)' which has an else clause
with braces, so the if clause needs braces too.


Lothar Waßmann

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

end of thread, other threads:[~2019-07-05 15:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-24 14:07 [PATCH] dmaengine: imx-sdma: fix use-after-free on probe error path Sven Van Asbroeck
2019-06-25  7:14 ` Robin Gong
2019-07-05  7:28 ` Vinod Koul
2019-07-05 12:26   ` Sven Van Asbroeck
2019-07-05 15:47     ` Lothar Waßmann
2019-07-05 12:46 ` Vinod Koul
2019-07-05 13:08   ` Sven Van Asbroeck
2019-07-05 13:10     ` Fabio Estevam
2019-07-05 13:10     ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).