Re: [PATCH v2] firmware_loader: fix device reference leak in firmware_upload_register()

[Guangshuo Li]

Hi Danilo, Russ,

Thanks for your reviewing.

On Tue, 5 May 2026 at 03:18, Danilo Krummrich <dakr@xxxxxxxxxx> wrote:
>
> On Sat Apr 18, 2026 at 9:02 AM CEST, Guangshuo Li wrote:
> > diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
> > index f59a7856934c..a6dab34b22d8 100644
> > --- a/drivers/base/firmware_loader/sysfs_upload.c
> > +++ b/drivers/base/firmware_loader/sysfs_upload.c
> > @@ -351,7 +351,8 @@ firmware_upload_register(struct module *module, struct device *parent,
> >       if (ret != 0) {
> >               if (ret > 0)
> >                       ret = -EINVAL;
> > -             goto free_fw_sysfs;
> > +             put_device(fw_dev);
> > +             goto exit_module_put;
> >       }
> >       fw_priv->is_paged_buf = true;
> >       fw_sysfs->fw_priv = fw_priv;
>
> I think this assignment comes too late, by calling put_device() we eventually
> end up in fw_upload_free() which expects this to be assigned already.
>
> I didn't think it through entirely, but can we just move the
>
>         fw_sysfs->fw_upload_priv = fw_upload_priv;
>
> assignment to after alloc_lookup_fw_priv(), so fw_dev_release() does not enter
> this path in the first place?

After manual re-review, I confirmed that the issue Danilo pointed out
does exist. In the current v2 patch, fw_sysfs->fw_upload_priv has
already been assigned before alloc_lookup_fw_priv() succeeds, while
fw_sysfs->fw_priv has not been assigned yet. Therefore, calling
put_device() on this failure path may eventually enter
fw_upload_free(), which expects fw_sysfs->fw_priv to be valid.

I will address this in a v3 patch.

Thanks,
Guangshuo