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@xxxxxxxxxx> 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@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.
>

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