[v1] mfd: ab8500: fix child device leaks on probe failure

[PATCH] mfd: ab8500: fix child device leaks on probe failure

Posted by Guangshuo Li 3 days, 7 hours ago

ab8500_probe() registers MFD child devices before completing the
remaining device initialization.

On AB8540, the main MFD device batch may be registered successfully
before registration of the cut-specific batch fails. In that case,
the probe returns without removing the child devices registered by
the earlier mfd_add_devices() call.

The same issue occurs when sysfs group creation fails after the MFD
children have been registered. Since the probe returns an error, there
is no later teardown path to unregister those devices.

Call mfd_remove_devices() on these error paths so that MFD child
devices successfully registered earlier in the probe are properly
unregistered and released.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fixes: cca69b67b3ba ("mfd: Export ab8500 chip id to sysfs")
Fixes: 9c717cf3fa16 ("mfd: ab8500-core: Add device for new RTC version for AB8540 cut2")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/mfd/ab8500-core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index f0bc0b5a6f4a..e675223b6023 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -1220,8 +1220,10 @@ static int ab8500_probe(struct platform_device *pdev)
 		ret = mfd_add_devices(ab8500->dev, 0, ab8500_devs,
 				ARRAY_SIZE(ab8500_devs), NULL,
 				0, ab8500->domain);
-	if (ret)
+	if (ret) {
+		mfd_remove_devices(ab8500->dev);
 		return ret;
+	}
 
 	/* Add battery management devices */
 	ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
@@ -1243,8 +1245,10 @@ static int ab8500_probe(struct platform_device *pdev)
 		ret = sysfs_create_group(&ab8500->dev->kobj,
 					 &ab8505_attr_group);
 
-	if (ret)
+	if (ret) {
 		dev_err(ab8500->dev, "error creating sysfs entries\n");
+		mfd_remove_devices(ab8500->dev);
+	}
 
 	return ret;
 }
-- 
2.43.0

Re: [PATCH] mfd: ab8500: fix child device leaks on probe failure

Posted by Lee Jones 2 days, 11 hours ago

On Mon, 21 Sep 2026, Guangshuo Li wrote:

> ab8500_probe() registers MFD child devices before completing the
> remaining device initialization.
> 
> On AB8540, the main MFD device batch may be registered successfully
> before registration of the cut-specific batch fails. In that case,
> the probe returns without removing the child devices registered by
> the earlier mfd_add_devices() call.
> 
> The same issue occurs when sysfs group creation fails after the MFD
> children have been registered. Since the probe returns an error, there
> is no later teardown path to unregister those devices.
> 
> Call mfd_remove_devices() on these error paths so that MFD child
> devices successfully registered earlier in the probe are properly
> unregistered and released.
> 
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
> 
> Fixes: cca69b67b3ba ("mfd: Export ab8500 chip id to sysfs")
> Fixes: 9c717cf3fa16 ("mfd: ab8500-core: Add device for new RTC version for AB8540 cut2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/mfd/ab8500-core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> index f0bc0b5a6f4a..e675223b6023 100644
> --- a/drivers/mfd/ab8500-core.c
> +++ b/drivers/mfd/ab8500-core.c
> @@ -1220,8 +1220,10 @@ static int ab8500_probe(struct platform_device *pdev)
>  		ret = mfd_add_devices(ab8500->dev, 0, ab8500_devs,
>  				ARRAY_SIZE(ab8500_devs), NULL,
>  				0, ab8500->domain);
> -	if (ret)
> +	if (ret) {
> +		mfd_remove_devices(ab8500->dev);

What about switching to devm_*() a.k.a. managed resources?

>  		return ret;
> +	}
>  
>  	/* Add battery management devices */
>  	ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> @@ -1243,8 +1245,10 @@ static int ab8500_probe(struct platform_device *pdev)
>  		ret = sysfs_create_group(&ab8500->dev->kobj,
>  					 &ab8505_attr_group);
>  
> -	if (ret)
> +	if (ret) {
>  		dev_err(ab8500->dev, "error creating sysfs entries\n");
> +		mfd_remove_devices(ab8500->dev);
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.43.0
> 

-- 
Lee Jones

Re: [PATCH] mfd: ab8500: fix child device leaks on probe failure

Posted by Guangshuo Li 2 days, 10 hours ago

Hi Lee,

Thanks.

On Tue, 22 Sept 2026 at 15:41, Lee Jones <lee@kernel.org> wrote:
>
> On Mon, 21 Sep 2026, Guangshuo Li wrote:
>
> > ab8500_probe() registers MFD child devices before completing the
> > remaining device initialization.
> >
> > On AB8540, the main MFD device batch may be registered successfully
> > before registration of the cut-specific batch fails. In that case,
> > the probe returns without removing the child devices registered by
> > the earlier mfd_add_devices() call.
> >
> > The same issue occurs when sysfs group creation fails after the MFD
> > children have been registered. Since the probe returns an error, there
> > is no later teardown path to unregister those devices.
> >
> > Call mfd_remove_devices() on these error paths so that MFD child
> > devices successfully registered earlier in the probe are properly
> > unregistered and released.
> >
> > The issue was identified by a static analysis tool I developed and
> > confirmed by manual review.
> >
> > Fixes: cca69b67b3ba ("mfd: Export ab8500 chip id to sysfs")
> > Fixes: 9c717cf3fa16 ("mfd: ab8500-core: Add device for new RTC version for AB8540 cut2")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> >  drivers/mfd/ab8500-core.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> > index f0bc0b5a6f4a..e675223b6023 100644
> > --- a/drivers/mfd/ab8500-core.c
> > +++ b/drivers/mfd/ab8500-core.c
> > @@ -1220,8 +1220,10 @@ static int ab8500_probe(struct platform_device *pdev)
> >               ret = mfd_add_devices(ab8500->dev, 0, ab8500_devs,
> >                               ARRAY_SIZE(ab8500_devs), NULL,
> >                               0, ab8500->domain);
> > -     if (ret)
> > +     if (ret) {
> > +             mfd_remove_devices(ab8500->dev);
>
> What about switching to devm_*() a.k.a. managed resources?
>
> >               return ret;
> > +     }
> >
> >       /* Add battery management devices */
> >       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> > @@ -1243,8 +1245,10 @@ static int ab8500_probe(struct platform_device *pdev)
> >               ret = sysfs_create_group(&ab8500->dev->kobj,
> >                                        &ab8505_attr_group);
> >
> > -     if (ret)
> > +     if (ret) {
> >               dev_err(ab8500->dev, "error creating sysfs entries\n");
> > +             mfd_remove_devices(ab8500->dev);
> > +     }
> >
> >       return ret;
> >  }
> > --
> > 2.43.0
> >
>
> --
> Lee Jones

Do you mean converting the MFD registrations in ab8500_probe() to
devm_mfd_add_devices(), so the children are cleaned up automatically on
probe failure, e.g.:

-       ret = mfd_add_devices(ab8500->dev, 0, ab8540_devs,
+       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8540_devs,
                              ARRAY_SIZE(ab8540_devs), NULL,
                              0, ab8500->domain);

        ...

-       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
+       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
                              ARRAY_SIZE(ab8500_bm_devs), NULL,
                              0, ab8500->domain);

and likewise for the other MFD batches?

If so, I'll rework the patch that way for v2.

Thanks,
Guangshuo

Re: [PATCH] mfd: ab8500: fix child device leaks on probe failure

Posted by Lee Jones 2 days, 3 hours ago

On Tue, 22 Sep 2026, Guangshuo Li wrote:

> Hi Lee,
> 
> Thanks.
> 
> On Tue, 22 Sept 2026 at 15:41, Lee Jones <lee@kernel.org> wrote:
> >
> > On Mon, 21 Sep 2026, Guangshuo Li wrote:
> >
> > > ab8500_probe() registers MFD child devices before completing the
> > > remaining device initialization.
> > >
> > > On AB8540, the main MFD device batch may be registered successfully
> > > before registration of the cut-specific batch fails. In that case,
> > > the probe returns without removing the child devices registered by
> > > the earlier mfd_add_devices() call.
> > >
> > > The same issue occurs when sysfs group creation fails after the MFD
> > > children have been registered. Since the probe returns an error, there
> > > is no later teardown path to unregister those devices.
> > >
> > > Call mfd_remove_devices() on these error paths so that MFD child
> > > devices successfully registered earlier in the probe are properly
> > > unregistered and released.
> > >
> > > The issue was identified by a static analysis tool I developed and
> > > confirmed by manual review.
> > >
> > > Fixes: cca69b67b3ba ("mfd: Export ab8500 chip id to sysfs")
> > > Fixes: 9c717cf3fa16 ("mfd: ab8500-core: Add device for new RTC version for AB8540 cut2")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > > ---
> > >  drivers/mfd/ab8500-core.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> > > index f0bc0b5a6f4a..e675223b6023 100644
> > > --- a/drivers/mfd/ab8500-core.c
> > > +++ b/drivers/mfd/ab8500-core.c
> > > @@ -1220,8 +1220,10 @@ static int ab8500_probe(struct platform_device *pdev)
> > >               ret = mfd_add_devices(ab8500->dev, 0, ab8500_devs,
> > >                               ARRAY_SIZE(ab8500_devs), NULL,
> > >                               0, ab8500->domain);
> > > -     if (ret)
> > > +     if (ret) {
> > > +             mfd_remove_devices(ab8500->dev);
> >
> > What about switching to devm_*() a.k.a. managed resources?
> >
> > >               return ret;
> > > +     }
> > >
> > >       /* Add battery management devices */
> > >       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> > > @@ -1243,8 +1245,10 @@ static int ab8500_probe(struct platform_device *pdev)
> > >               ret = sysfs_create_group(&ab8500->dev->kobj,
> > >                                        &ab8505_attr_group);
> > >
> > > -     if (ret)
> > > +     if (ret) {
> > >               dev_err(ab8500->dev, "error creating sysfs entries\n");
> > > +             mfd_remove_devices(ab8500->dev);
> > > +     }
> > >
> > >       return ret;
> > >  }
> > > --
> > > 2.43.0
> > >
> >
> > --
> > Lee Jones
> 
> Do you mean converting the MFD registrations in ab8500_probe() to
> devm_mfd_add_devices(), so the children are cleaned up automatically on
> probe failure, e.g.:
> 
> -       ret = mfd_add_devices(ab8500->dev, 0, ab8540_devs,
> +       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8540_devs,
>                               ARRAY_SIZE(ab8540_devs), NULL,
>                               0, ab8500->domain);
> 
>         ...
> 
> -       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> +       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
>                               ARRAY_SIZE(ab8500_bm_devs), NULL,
>                               0, ab8500->domain);
> 
> and likewise for the other MFD batches?
> 
> If so, I'll rework the patch that way for v2.

Yes.

-- 
Lee Jones

Re: [PATCH] mfd: ab8500: fix child device leaks on probe failure

Posted by Guangshuo Li 4 hours ago

Thanks, Lee.

On Tue, 22 Sept 2026 at 23:06, Lee Jones <lee@kernel.org> wrote:
>
> On Tue, 22 Sep 2026, Guangshuo Li wrote:
>
> > Hi Lee,
> >
> > Thanks.
> >
> > On Tue, 22 Sept 2026 at 15:41, Lee Jones <lee@kernel.org> wrote:
> > >
> > > On Mon, 21 Sep 2026, Guangshuo Li wrote:
> > >
> > > > ab8500_probe() registers MFD child devices before completing the
> > > > remaining device initialization.
> > > >
> > > > On AB8540, the main MFD device batch may be registered successfully
> > > > before registration of the cut-specific batch fails. In that case,
> > > > the probe returns without removing the child devices registered by
> > > > the earlier mfd_add_devices() call.
> > > >
> > > > The same issue occurs when sysfs group creation fails after the MFD
> > > > children have been registered. Since the probe returns an error, there
> > > > is no later teardown path to unregister those devices.
> > > >
> > > > Call mfd_remove_devices() on these error paths so that MFD child
> > > > devices successfully registered earlier in the probe are properly
> > > > unregistered and released.
> > > >
> > > > The issue was identified by a static analysis tool I developed and
> > > > confirmed by manual review.
> > > >
> > > > Fixes: cca69b67b3ba ("mfd: Export ab8500 chip id to sysfs")
> > > > Fixes: 9c717cf3fa16 ("mfd: ab8500-core: Add device for new RTC version for AB8540 cut2")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > > > ---
> > > >  drivers/mfd/ab8500-core.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
> > > > index f0bc0b5a6f4a..e675223b6023 100644
> > > > --- a/drivers/mfd/ab8500-core.c
> > > > +++ b/drivers/mfd/ab8500-core.c
> > > > @@ -1220,8 +1220,10 @@ static int ab8500_probe(struct platform_device *pdev)
> > > >               ret = mfd_add_devices(ab8500->dev, 0, ab8500_devs,
> > > >                               ARRAY_SIZE(ab8500_devs), NULL,
> > > >                               0, ab8500->domain);
> > > > -     if (ret)
> > > > +     if (ret) {
> > > > +             mfd_remove_devices(ab8500->dev);
> > >
> > > What about switching to devm_*() a.k.a. managed resources?
> > >
> > > >               return ret;
> > > > +     }
> > > >
> > > >       /* Add battery management devices */
> > > >       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> > > > @@ -1243,8 +1245,10 @@ static int ab8500_probe(struct platform_device *pdev)
> > > >               ret = sysfs_create_group(&ab8500->dev->kobj,
> > > >                                        &ab8505_attr_group);
> > > >
> > > > -     if (ret)
> > > > +     if (ret) {
> > > >               dev_err(ab8500->dev, "error creating sysfs entries\n");
> > > > +             mfd_remove_devices(ab8500->dev);
> > > > +     }
> > > >
> > > >       return ret;
> > > >  }
> > > > --
> > > > 2.43.0
> > > >
> > >
> > > --
> > > Lee Jones
> >
> > Do you mean converting the MFD registrations in ab8500_probe() to
> > devm_mfd_add_devices(), so the children are cleaned up automatically on
> > probe failure, e.g.:
> >
> > -       ret = mfd_add_devices(ab8500->dev, 0, ab8540_devs,
> > +       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8540_devs,
> >                               ARRAY_SIZE(ab8540_devs), NULL,
> >                               0, ab8500->domain);
> >
> >         ...
> >
> > -       ret = mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> > +       ret = devm_mfd_add_devices(ab8500->dev, 0, ab8500_bm_devs,
> >                               ARRAY_SIZE(ab8500_bm_devs), NULL,
> >                               0, ab8500->domain);
> >
> > and likewise for the other MFD batches?
> >
> > If so, I'll rework the patch that way for v2.
>
> Yes.
>
> --
> Lee Jones

I'll convert all the MFD registrations to devm_mfd_add_devices() and
send a v2.

Thanks,
Guangshuo