[PATCH] soc: mpfs: use kref cleanup on probe failure

[PATCH] soc: mpfs: use kref cleanup on probe failure

[Guangshuo Li]

After kref_init(), the device_get_match_data() failure path jumps to
out_free_channel. That path frees the mailbox channel and then falls
through to kfree(), bypassing mpfs_sys_controller_put() for the initial
reference.

Call mpfs_sys_controller_put() on that path and return immediately. The
existing kref release callback then performs the matching cleanup, and
the return prevents a second free through out_free. This is the minimal
change needed to keep the initialized lifetime balanced.

Fixes: 75ef23397558 ("soc: microchip: mpfs-sys-controller: fix resource leak on probe error")
---
 drivers/soc/microchip/mpfs-sys-controller.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
index 0400a01b2338..379e4f649faa 100644
--- a/drivers/soc/microchip/mpfs-sys-controller.c
+++ b/drivers/soc/microchip/mpfs-sys-controller.c
@@ -174,7 +174,8 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
 	return 0;
 
 out_free_channel:
-	mbox_free_channel(sys_controller->chan);
+	mpfs_sys_controller_put(sys_controller);
+	return ret;
 out_free:
 	kfree(sys_controller);
 	return ret;

base-commit: f03c39de3a1307371a4032757cd1732e91087c7d
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: mpfs: use kref cleanup on probe failure

[Felix Gu]

Hi Guangshuo,

On Thu, Sep 24, 2026 at 7:01 PM Guangshuo Li <lgs201920130244@gmail.com> wrote:
>
> After kref_init(), the device_get_match_data() failure path jumps to
> out_free_channel. That path frees the mailbox channel and then falls
> through to kfree(), bypassing mpfs_sys_controller_put() for the initial
> reference.
>
> Call mpfs_sys_controller_put() on that path and return immediately. The
> existing kref release callback then performs the matching cleanup, and
> the return prevents a second free through out_free. This is the minimal
> change needed to keep the initialized lifetime balanced.
>

I don't think this is a fix.  mpfs_sys_controller_put() does the same
mbox_free_channel() + kfree() the current path already does.

> Fixes: 75ef23397558 ("soc: microchip: mpfs-sys-controller: fix resource leak on probe error")
> ---
>  drivers/soc/microchip/mpfs-sys-controller.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
> index 0400a01b2338..379e4f649faa 100644
> --- a/drivers/soc/microchip/mpfs-sys-controller.c
> +++ b/drivers/soc/microchip/mpfs-sys-controller.c
> @@ -174,7 +174,8 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
>         return 0;
>
>  out_free_channel:
> -       mbox_free_channel(sys_controller->chan);
> +       mpfs_sys_controller_put(sys_controller);
> +       return ret;

If you do keep this, the out_free_channel label no longer fits.

Thanks,
Felix

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: mpfs: use kref cleanup on probe failure

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2402 bytes --]

On Fri, Sep 25, 2026 at 12:04:16AM +0800, Felix Gu wrote:
> Hi Guangshuo,
> 
> On Thu, Sep 24, 2026 at 7:01 PM Guangshuo Li <lgs201920130244@gmail.com> wrote:
> >
> > After kref_init(), the device_get_match_data() failure path jumps to
> > out_free_channel. That path frees the mailbox channel and then falls
> > through to kfree(), bypassing mpfs_sys_controller_put() for the initial
> > reference.
> >
> > Call mpfs_sys_controller_put() on that path and return immediately. The
> > existing kref release callback then performs the matching cleanup, and
> > the return prevents a second free through out_free. This is the minimal
> > change needed to keep the initialized lifetime balanced.
> >
> 
> I don't think this is a fix.  mpfs_sys_controller_put() does the same
> mbox_free_channel() + kfree() the current path already does.

Why would it not be a fix? Maybe you mixed up the delete and put?

static void mpfs_sys_controller_delete(struct kref *kref)
{
	struct mpfs_sys_controller *sys_controller =
		container_of(kref, struct mpfs_sys_controller, consumers);

	mbox_free_channel(sys_controller->chan);
	kfree(sys_controller);
}

static void mpfs_sys_controller_put(void *data)
{
	struct mpfs_sys_controller *sys_controller = data;

	kref_put(&sys_controller->consumers, mpfs_sys_controller_delete);
}

the kref_put() that wraps mpfs_sys_controller_delete() decreases the
refcount and is the matching action for the kref_init() during probe.


> > Fixes: 75ef23397558 ("soc: microchip: mpfs-sys-controller: fix resource leak on probe error")

Unfortunately, without a signoff, I cannot apply this.

> > ---
> >  drivers/soc/microchip/mpfs-sys-controller.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
> > index 0400a01b2338..379e4f649faa 100644
> > --- a/drivers/soc/microchip/mpfs-sys-controller.c
> > +++ b/drivers/soc/microchip/mpfs-sys-controller.c
> > @@ -174,7 +174,8 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
> >         return 0;
> >
> >  out_free_channel:
> > -       mbox_free_channel(sys_controller->chan);
> > +       mpfs_sys_controller_put(sys_controller);
> > +       return ret;
> 
> If you do keep this, the out_free_channel label no longer fits.

True.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] soc: mpfs: use kref cleanup on probe failure

[Guangshuo Li]

Hi Conor,

Thanks for pointing this out.

On Sat, 3 Oct 2026 at 01:22, Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 12:04:16AM +0800, Felix Gu wrote:
> > Hi Guangshuo,
> >
> > On Thu, Sep 24, 2026 at 7:01 PM Guangshuo Li <lgs201920130244@gmail.com> wrote:
> > >
> > > After kref_init(), the device_get_match_data() failure path jumps to
> > > out_free_channel. That path frees the mailbox channel and then falls
> > > through to kfree(), bypassing mpfs_sys_controller_put() for the initial
> > > reference.
> > >
> > > Call mpfs_sys_controller_put() on that path and return immediately. The
> > > existing kref release callback then performs the matching cleanup, and
> > > the return prevents a second free through out_free. This is the minimal
> > > change needed to keep the initialized lifetime balanced.
> > >
> >
> > I don't think this is a fix.  mpfs_sys_controller_put() does the same
> > mbox_free_channel() + kfree() the current path already does.
>
> Why would it not be a fix? Maybe you mixed up the delete and put?
>
> static void mpfs_sys_controller_delete(struct kref *kref)
> {
>         struct mpfs_sys_controller *sys_controller =
>                 container_of(kref, struct mpfs_sys_controller, consumers);
>
>         mbox_free_channel(sys_controller->chan);
>         kfree(sys_controller);
> }
>
> static void mpfs_sys_controller_put(void *data)
> {
>         struct mpfs_sys_controller *sys_controller = data;
>
>         kref_put(&sys_controller->consumers, mpfs_sys_controller_delete);
> }
>
> the kref_put() that wraps mpfs_sys_controller_delete() decreases the
> refcount and is the matching action for the kref_init() during probe.
>
>
> > > Fixes: 75ef23397558 ("soc: microchip: mpfs-sys-controller: fix resource leak on probe error")
>
> Unfortunately, without a signoff, I cannot apply this.
>
> > > ---
> > >  drivers/soc/microchip/mpfs-sys-controller.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/soc/microchip/mpfs-sys-controller.c b/drivers/soc/microchip/mpfs-sys-controller.c
> > > index 0400a01b2338..379e4f649faa 100644
> > > --- a/drivers/soc/microchip/mpfs-sys-controller.c
> > > +++ b/drivers/soc/microchip/mpfs-sys-controller.c
> > > @@ -174,7 +174,8 @@ static int mpfs_sys_controller_probe(struct platform_device *pdev)
> > >         return 0;
> > >
> > >  out_free_channel:
> > > -       mbox_free_channel(sys_controller->chan);
> > > +       mpfs_sys_controller_put(sys_controller);
> > > +       return ret;
> >
> > If you do keep this, the out_free_channel label no longer fits.
>
> True.
>

The missing Signed-off-by tag was our mistake. I will add it and send
a v2, together with the error label rename suggested by Felix.

Sorry for the oversight, and thanks for the review.

Best regards,
Guangshuo

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv