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

[Conor Dooley]

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@xxxxxxxxx> 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: signature.asc
Description: PGP signature