mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure
@ 2026-09-21 10:40 Guangshuo Li
  2026-09-21 11:05 ` Mika Westerberg
  0 siblings, 1 reply; 5+ messages in thread
From: Guangshuo Li @ 2026-09-21 10:40 UTC (permalink / raw)
  To: Lee Jones, Mika Westerberg, Andy Shevchenko, mfd, linux-kernel
  Cc: Guangshuo Li, stable

intel_lpss_probe() may register an iDMA MFD child before registering
the main controller cell. If the iDMA child is registered successfully
but the subsequent mfd_add_devices() call for the main cell fails, the
probe error path returns without removing the already registered iDMA
device.

Since the normal remove callback is not called after a failed probe,
the iDMA platform device remains registered and is never released.

Call mfd_remove_devices() on the probe error path to remove any MFD
children that were successfully registered before the main cell
registration failed.

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

Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/mfd/intel-lpss.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 63d6694f7145..657b1e3fb896 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -455,6 +455,7 @@ int intel_lpss_probe(struct device *dev,
 	return 0;
 
 err_remove_ltr:
+	mfd_remove_devices(dev);
 	intel_lpss_debugfs_remove(lpss);
 	intel_lpss_ltr_hide(lpss);
 	intel_lpss_unregister_clock(lpss);
-- 
2.43.0


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

* Re: [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure
  2026-09-21 10:40 [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure Guangshuo Li
@ 2026-09-21 11:05 ` Mika Westerberg
  2026-09-22  7:59   ` Guangshuo Li
  0 siblings, 1 reply; 5+ messages in thread
From: Mika Westerberg @ 2026-09-21 11:05 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: Lee Jones, Andy Shevchenko, mfd, linux-kernel, stable

Hi,

On Mon, Sep 21, 2026 at 06:40:58PM +0800, Guangshuo Li wrote:
> intel_lpss_probe() may register an iDMA MFD child before registering
> the main controller cell. If the iDMA child is registered successfully
> but the subsequent mfd_add_devices() call for the main cell fails, the
> probe error path returns without removing the already registered iDMA
> device.
> 
> Since the normal remove callback is not called after a failed probe,
> the iDMA platform device remains registered and is never released.
> 
> Call mfd_remove_devices() on the probe error path to remove any MFD
> children that were successfully registered before the main cell
> registration failed.

I think this could use devm_mfd_add_devices() instead.

> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
> 
> Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/mfd/intel-lpss.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 63d6694f7145..657b1e3fb896 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -455,6 +455,7 @@ int intel_lpss_probe(struct device *dev,
>  	return 0;
>  
>  err_remove_ltr:
> +	mfd_remove_devices(dev);
>  	intel_lpss_debugfs_remove(lpss);
>  	intel_lpss_ltr_hide(lpss);
>  	intel_lpss_unregister_clock(lpss);
> -- 
> 2.43.0

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

* Re: [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure
  2026-09-21 11:05 ` Mika Westerberg
@ 2026-09-22  7:59   ` Guangshuo Li
  2026-09-22 12:29     ` Andy Shevchenko
  0 siblings, 1 reply; 5+ messages in thread
From: Guangshuo Li @ 2026-09-22  7:59 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: Lee Jones, Andy Shevchenko, mfd, linux-kernel, stable

Hi Mika,

Thanks for the suggestion.

On Mon, 21 Sept 2026 at 19:05, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Mon, Sep 21, 2026 at 06:40:58PM +0800, Guangshuo Li wrote:
> > intel_lpss_probe() may register an iDMA MFD child before registering
> > the main controller cell. If the iDMA child is registered successfully
> > but the subsequent mfd_add_devices() call for the main cell fails, the
> > probe error path returns without removing the already registered iDMA
> > device.
> >
> > Since the normal remove callback is not called after a failed probe,
> > the iDMA platform device remains registered and is never released.
> >
> > Call mfd_remove_devices() on the probe error path to remove any MFD
> > children that were successfully registered before the main cell
> > registration failed.
>
> I think this could use devm_mfd_add_devices() instead.
>
> > The issue was identified by a static analysis tool I developed and
> > confirmed by manual review.
> >
> > Fixes: 4b45efe85263 ("mfd: Add support for Intel Sunrisepoint LPSS devices")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> >  drivers/mfd/intel-lpss.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> > index 63d6694f7145..657b1e3fb896 100644
> > --- a/drivers/mfd/intel-lpss.c
> > +++ b/drivers/mfd/intel-lpss.c
> > @@ -455,6 +455,7 @@ int intel_lpss_probe(struct device *dev,
> >       return 0;
> >
> >  err_remove_ltr:
> > +     mfd_remove_devices(dev);
> >       intel_lpss_debugfs_remove(lpss);
> >       intel_lpss_ltr_hide(lpss);
> >       intel_lpss_unregister_clock(lpss);
> > --
> > 2.43.0

Do you mean converting both MFD registrations to devm_mfd_add_devices()
and dropping the explicit mfd_remove_devices() from the remove path, like
this?

        if (intel_lpss_has_idma(lpss)) {
-               ret = mfd_add_devices(dev, lpss->devid, &intel_lpss_idma64_cell,
-                                     1, info->mem, info->irq, NULL);
+               ret = devm_mfd_add_devices(dev, lpss->devid,
+                                          &intel_lpss_idma64_cell, 1,
+                                          info->mem, info->irq, NULL);
                if (ret)
                        dev_warn(dev, "Failed to add %s, fallback to PIO\n",
                                 LPSS_IDMA64_DRIVER_NAME);
        }

-       ret = mfd_add_devices(dev, lpss->devid, lpss->cell,
-                             1, info->mem, info->irq, NULL);
+       ret = devm_mfd_add_devices(dev, lpss->devid, lpss->cell,
+                                  1, info->mem, info->irq, NULL);
        if (ret)
                goto err_remove_ltr;

...

 void intel_lpss_remove(struct device *dev)
 {
        struct intel_lpss *lpss = dev_get_drvdata(dev);

-       mfd_remove_devices(dev);
        intel_lpss_debugfs_remove(lpss);
        intel_lpss_ltr_hide(lpss);
        intel_lpss_unregister_clock(lpss);
        ida_free(&intel_lpss_devid_ida, lpss->devid);
 }

Would this be the preferred change?

Thanks,
Guangshuo

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

* Re: [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure
  2026-09-22  7:59   ` Guangshuo Li
@ 2026-09-22 12:29     ` Andy Shevchenko
  2026-09-22 12:33       ` Mika Westerberg
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2026-09-22 12:29 UTC (permalink / raw)
  To: Guangshuo Li; +Cc: Mika Westerberg, Lee Jones, mfd, linux-kernel, stable

On Tue, Sep 22, 2026 at 03:59:32PM +0800, Guangshuo Li wrote:
> On Mon, 21 Sept 2026 at 19:05, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Mon, Sep 21, 2026 at 06:40:58PM +0800, Guangshuo Li wrote:

> > I think this could use devm_mfd_add_devices() instead.
> >
> > > The issue was identified by a static analysis tool I developed and
> > > confirmed by manual review.

> Do you mean converting both MFD registrations to devm_mfd_add_devices()
> and dropping the explicit mfd_remove_devices() from the remove path, like
> this?

Won't work properly. The rule of thumb, it should be no 'goto' after devm_*() call.

> Would this be the preferred change?

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure
  2026-09-22 12:29     ` Andy Shevchenko
@ 2026-09-22 12:33       ` Mika Westerberg
  0 siblings, 0 replies; 5+ messages in thread
From: Mika Westerberg @ 2026-09-22 12:33 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Guangshuo Li, Lee Jones, mfd, linux-kernel, stable

On Tue, Sep 22, 2026 at 03:29:24PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 22, 2026 at 03:59:32PM +0800, Guangshuo Li wrote:
> > On Mon, 21 Sept 2026 at 19:05, Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > > On Mon, Sep 21, 2026 at 06:40:58PM +0800, Guangshuo Li wrote:
> 
> > > I think this could use devm_mfd_add_devices() instead.
> > >
> > > > The issue was identified by a static analysis tool I developed and
> > > > confirmed by manual review.
> 
> > Do you mean converting both MFD registrations to devm_mfd_add_devices()
> > and dropping the explicit mfd_remove_devices() from the remove path, like
> > this?
> 
> Won't work properly. The rule of thumb, it should be no 'goto' after devm_*() call.

hmm, isn't the unwind done in driver core and has nothing to do with goto?
Maybe things have changed since I last time looked? ;-)

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

end of thread, other threads:[~2026-09-22 12:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-21 10:40 [PATCH] mfd: intel-lpss: fix iDMA device leak on probe failure Guangshuo Li
2026-09-21 11:05 ` Mika Westerberg
2026-09-22  7:59   ` Guangshuo Li
2026-09-22 12:29     ` Andy Shevchenko
2026-09-22 12:33       ` Mika Westerberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®