Re: [PATCH] nouveau/firmware: fix memory leak on BL load failure

[Dawei Feng <dawei.feng@seu.edu.cn>]

Hi, Timur,

On Fri, Jun 05, 2026 at 06:22:41PM +0000, Timur Tabi wrote:
> Ah yes, you're right.
>
> So now I think a better fix might be to have two different `blob`
> variables, so that there is no longer any confusion. Because right now,
> the nvkm_firmware_put() call at the end of the function releases a
> different `blob` depending on whether `bl` is NULL or not.
>
> What do you think about this:
>
>     nvkm_firmware_put(blob);
>     if (bl) {
>             const struct firmware *blob_bl;
>
>             ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob_bl);
>             if (ret)
>                     goto done;
>             ...
>             nvkm_firmware_put(blob_bl);
>             if (!fw->boot)
>                     ret = -ENOMEM;
>     } else {
>             fw->boot_addr = fw->nmem_base;
>     }
>
> done:
>     if (ret)
>             nvkm_falcon_fw_dtor(fw);
>
>     return ret;

Yes, using a separate pointer `blob_bl` for the bootloader firmware is a
cleaner approach. 

However, we must keep the final nvkm_firmware_put(blob) under the done
label. Moving it earlier would cause memory leaks in prior error paths
like nvkm_falcon_fw_ctor(), which jump directly to done. 

A safer approach is to manage blob_bl locally inside the if (bl) block,
while leaving the original blob cleanup at the end. 

What do you think about this:

        if (bl) {
                const struct firmware *blob_bl;

                ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob_bl);
                if (ret)
                        goto done;

                ...
                nvkm_firmware_put(blob_bl);
                if (!fw->boot)
                        ret = -ENOMEM;
        } else {
                fw->boot_addr = fw->nmem_base;
        }

done:
        if (ret)
                nvkm_falcon_fw_dtor(fw);

        nvkm_firmware_put(blob);
        return ret;

Regards,
Dawei