1. Patchew
  2. linux
  3. View series

[PATCHv3] dmaengine: fsl_raid: check fsl_re_chan_probe() return value

Rosen Penev posted 1 patch 3 days, 1 hour ago 
drivers/dma/fsl_raid.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
[PATCHv3] dmaengine: fsl_raid: check fsl_re_chan_probe() return value
Posted by Rosen Penev 3 days, 1 hour ago 
fsl_re_probe() ignores the return value of fsl_re_chan_probe() and
unconditionally increments total_chans. When a channel fails to probe
(for example, an IRQ mapping failure) its re_jrs[] slot is left NULL, yet
total_chans still advances, so fsl_re_remove_chan() later dereferences the
NULL pointer during device removal.

Check return value and only count successfully probed channels, and guard
fsl_re_remove() against NULL entries.

Fixes: ad80da658bbc ("dmaengine: Driver support for FSL RaidEngine device.")
Assisted-by: opencode:hy3-free
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v3: fix sashiko review
 v2: fix description
 drivers/dma/fsl_raid.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
index bfaef6245695..66de27eb4365 100644
--- a/drivers/dma/fsl_raid.c
+++ b/drivers/dma/fsl_raid.c
@@ -841,13 +841,30 @@ static int fsl_re_probe(struct platform_device *ofdev)
 		for_each_child_of_node(np, child) {
 			rc = of_device_is_compatible(child,
 					     "fsl,raideng-v1.0-job-ring");
+			if (!rc)
+				continue;
+
+			if (ridx >= FSL_RE_MAX_CHANS) {
+				dev_warn(dev,
+					"too many job rings, max %d\n",
+					FSL_RE_MAX_CHANS);
+				of_node_put(child);
+				goto done;
+			}
+
+			rc = fsl_re_chan_probe(ofdev, child, ridx, off);
 			if (rc) {
-				fsl_re_chan_probe(ofdev, child, ridx++, off);
-				re_priv->total_chans++;
+				dev_err(dev,
+					"job ring %d probe failed: %d\n",
+					ridx, rc);
+				continue;
 			}
+			ridx++;
+			re_priv->total_chans++;
 		}
 	}
 
+done:
 	dma_async_device_register(dma_dev);
 
 	return 0;
@@ -875,7 +892,8 @@ static void fsl_re_remove(struct platform_device *ofdev)
 
 	/* Cleanup chan related memory areas */
 	for (i = 0; i < re_priv->total_chans; i++)
-		fsl_re_remove_chan(re_priv->re_jrs[i]);
+		if (re_priv->re_jrs[i])
+			fsl_re_remove_chan(re_priv->re_jrs[i]);
 
 	/* Unregister the driver */
 	dma_async_device_unregister(&re_priv->dma_dev);
-- 
2.55.0
Re: [PATCHv3] dmaengine: fsl_raid: check fsl_re_chan_probe() return value
Posted by Frank Li 2 days, 23 hours ago 
On Tue, Jul 21, 2026 at 03:18:07PM -0700, Rosen Penev wrote:
> fsl_re_probe() ignores the return value of fsl_re_chan_probe() and
> unconditionally increments total_chans. When a channel fails to probe
> (for example, an IRQ mapping failure) its re_jrs[] slot is left NULL, yet
> total_chans still advances, so fsl_re_remove_chan() later dereferences the
> NULL pointer during device removal.
>
> Check return value and only count successfully probed channels, and guard
> fsl_re_remove() against NULL entries.
>
> Fixes: ad80da658bbc ("dmaengine: Driver support for FSL RaidEngine device.")
> Assisted-by: opencode:hy3-free
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  v3: fix sashiko review
>  v2: fix description
>  drivers/dma/fsl_raid.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> index bfaef6245695..66de27eb4365 100644
> --- a/drivers/dma/fsl_raid.c
> +++ b/drivers/dma/fsl_raid.c
> @@ -841,13 +841,30 @@ static int fsl_re_probe(struct platform_device *ofdev)
>  		for_each_child_of_node(np, child) {
>  			rc = of_device_is_compatible(child,
>  					     "fsl,raideng-v1.0-job-ring");
> +			if (!rc)
> +				continue;
> +
> +			if (ridx >= FSL_RE_MAX_CHANS) {
> +				dev_warn(dev,
> +					"too many job rings, max %d\n",
> +					FSL_RE_MAX_CHANS);
> +				of_node_put(child);
> +				goto done;

does break work here?

Frank
> +			}
> +
> +			rc = fsl_re_chan_probe(ofdev, child, ridx, off);
>  			if (rc) {
> -				fsl_re_chan_probe(ofdev, child, ridx++, off);
> -				re_priv->total_chans++;
> +				dev_err(dev,
> +					"job ring %d probe failed: %d\n",
> +					ridx, rc);
> +				continue;
>  			}
> +			ridx++;
> +			re_priv->total_chans++;
>  		}
>  	}
>
> +done:
>  	dma_async_device_register(dma_dev);
>
>  	return 0;
> @@ -875,7 +892,8 @@ static void fsl_re_remove(struct platform_device *ofdev)
>
>  	/* Cleanup chan related memory areas */
>  	for (i = 0; i < re_priv->total_chans; i++)
> -		fsl_re_remove_chan(re_priv->re_jrs[i]);
> +		if (re_priv->re_jrs[i])
> +			fsl_re_remove_chan(re_priv->re_jrs[i]);
>
>  	/* Unregister the driver */
>  	dma_async_device_unregister(&re_priv->dma_dev);
> --
> 2.55.0
>
Re: [PATCHv3] dmaengine: fsl_raid: check fsl_re_chan_probe() return value
Posted by Rosen Penev 2 days, 22 hours ago 
On Tue, Jul 21, 2026 at 5:00 PM Frank Li <Frank.li@oss.nxp.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:18:07PM -0700, Rosen Penev wrote:
> > fsl_re_probe() ignores the return value of fsl_re_chan_probe() and
> > unconditionally increments total_chans. When a channel fails to probe
> > (for example, an IRQ mapping failure) its re_jrs[] slot is left NULL, yet
> > total_chans still advances, so fsl_re_remove_chan() later dereferences the
> > NULL pointer during device removal.
> >
> > Check return value and only count successfully probed channels, and guard
> > fsl_re_remove() against NULL entries.
> >
> > Fixes: ad80da658bbc ("dmaengine: Driver support for FSL RaidEngine device.")
> > Assisted-by: opencode:hy3-free
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> >  v3: fix sashiko review
> >  v2: fix description
> >  drivers/dma/fsl_raid.c | 24 +++++++++++++++++++++---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/dma/fsl_raid.c b/drivers/dma/fsl_raid.c
> > index bfaef6245695..66de27eb4365 100644
> > --- a/drivers/dma/fsl_raid.c
> > +++ b/drivers/dma/fsl_raid.c
> > @@ -841,13 +841,30 @@ static int fsl_re_probe(struct platform_device *ofdev)
> >               for_each_child_of_node(np, child) {
> >                       rc = of_device_is_compatible(child,
> >                                            "fsl,raideng-v1.0-job-ring");
> > +                     if (!rc)
> > +                             continue;
> > +
> > +                     if (ridx >= FSL_RE_MAX_CHANS) {
> > +                             dev_warn(dev,
> > +                                     "too many job rings, max %d\n",
> > +                                     FSL_RE_MAX_CHANS);
> > +                             of_node_put(child);
> > +                             goto done;
>
> does break work here?
Oh I see what's going on. A different commit I have adds another goto
label in this function. I'll rework.
>
> Frank
> > +                     }
> > +
> > +                     rc = fsl_re_chan_probe(ofdev, child, ridx, off);
> >                       if (rc) {
> > -                             fsl_re_chan_probe(ofdev, child, ridx++, off);
> > -                             re_priv->total_chans++;
> > +                             dev_err(dev,
> > +                                     "job ring %d probe failed: %d\n",
> > +                                     ridx, rc);
> > +                             continue;
> >                       }
> > +                     ridx++;
> > +                     re_priv->total_chans++;
> >               }
> >       }
> >
> > +done:
> >       dma_async_device_register(dma_dev);
> >
> >       return 0;
> > @@ -875,7 +892,8 @@ static void fsl_re_remove(struct platform_device *ofdev)
> >
> >       /* Cleanup chan related memory areas */
> >       for (i = 0; i < re_priv->total_chans; i++)
> > -             fsl_re_remove_chan(re_priv->re_jrs[i]);
> > +             if (re_priv->re_jrs[i])
> > +                     fsl_re_remove_chan(re_priv->re_jrs[i]);
> >
> >       /* Unregister the driver */
> >       dma_async_device_unregister(&re_priv->dma_dev);
> > --
> > 2.55.0
> >

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.