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

[Timur Tabi <ttabi@nvidia.com>]

On Mon, 2026-06-08 at 21:52 +0800, Dawei Feng wrote:
> Hi Timur,
> 
> On Fri, Jun 05, 2026 at 06:22:41PM +0000, Timur Tabi wrote:
> > I think it would be cleaner to instead delete this
> > nvkm_firmware_put(blob) call here, and just rely on the call to
> > nvkm_firmware_put() at the end of nvkm_falcon_fw_ctor_hs(). Then you
> > won't need "blob = NULL".
> 
> Thanks for your review.
> 
> I don't think we can drop the nvkm_firmware_put(blob) here. At this
> point, blob still points to the image firmware loaded at the beginning of
> nvkm_falcon_fw_ctor_hs(). The later nvkm_firmware_load_name(..., &blob)
> call overwrites blob with the bootloader firmware pointer on success.
> 
> If we only rely on the final nvkm_firmware_put(blob), the success path
> would release the bootloader firmware, but the original image firmware
> pointer would be lost and leaked.

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;

This way, there is no confusion between the two blobs, and the bootloader blob exists only inside
the if-block that needs it.