If loading the HS bootloader blob fails, nvkm_falcon_fw_ctor_hs() returns
immediately. This skips the common cleanup path and leaks the firmware
state allocated by nvkm_falcon_fw_ctor() and nvkm_falcon_fw_sign().
Fix this by routing the load failure to the 'done' label so
nvkm_falcon_fw_dtor() can properly clean up the partially initialized
state. Keep the original image firmware in 'blob' until the common
cleanup path, and use a separate 'blob_bl' pointer for the bootloader
firmware so it can be released immediately after the bootloader data has
been copied.
The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still present in
v7.1-rc6.
An x86_64 allyesconfig build showed no new warnings. As we do not have a
supported NVIDIA GPU with the required firmware to test this path, no
runtime testing was able to be performed.
Fixes: 2541626cfb79 ("drm/nouveau/acr: use common falcon HS FW code for ACR
FWs")
Cc: [email protected]
Signed-off-by: Zilin Guan <[email protected]>
Signed-off-by: Dawei Feng <[email protected]>
---
Changes in v2:
- Use a separate bootloader firmware pointer instead of reusing 'blob'.
- Keep the original image firmware release in the common cleanup path.
drivers/gpu/drm/nouveau/nvkm/falcon/fw.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
index 4e8b3f1c7e25..063469ee462f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
+++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c
@@ -277,19 +277,20 @@ nvkm_falcon_fw_ctor_hs(const struct nvkm_falcon_fw_func
*func, const char *name,
fw->dmem_sign = loc - lhdr->data_dma_base;
if (bl) {
- nvkm_firmware_put(blob);
+ const struct firmware *blob_bl;
- ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob);
+ ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob_bl);
if (ret)
- return ret;
+ goto done;
- hdr = nvfw_bin_hdr(subdev, blob->data);
- desc = nvfw_bl_desc(subdev, blob->data + hdr->header_offset);
+ hdr = nvfw_bin_hdr(subdev, blob_bl->data);
+ desc = nvfw_bl_desc(subdev, blob_bl->data + hdr->header_offset);
fw->boot_addr = desc->start_tag << 8;
fw->boot_size = desc->code_size;
- fw->boot = kmemdup(blob->data + hdr->data_offset +
desc->code_off,
+ fw->boot = kmemdup(blob_bl->data + hdr->data_offset +
desc->code_off,
fw->boot_size, GFP_KERNEL);
+ nvkm_firmware_put(blob_bl);
if (!fw->boot)
ret = -ENOMEM;
} else {
--
2.34.1
