⬢ DragonFlyBSD Kernel Audit
DF-4774 / fix.diff
← back to finding ↓ download raw
fix.diff — DF-4774: bound the SMU firmware payload pointer/length against the
actual firmware blob in amdgpu_cgs_get_firmware_info() before handing
info->kptr / info->image_size to consumers (smu7/smu8/tonga-family/vega
upload loops).  amdgpu_ucode_validate() only proves
datasize == hdr->size_bytes (self-declared); it says nothing about
ucode_array_offset_bytes / ucode_size_bytes, so the powerplay upload loop
(smu7_smumgr.c:470-471) reads up to SMU7_SMC_SIZE bytes from
fw->data + attacker-chosen 32-bit offset.

Authored against the read-only sys/ tree; never applied there.

diff --git a/sys/dev/drm/amd/amdgpu/amdgpu_cgs.c b/sys/dev/drm/amd/amdgpu/amdgpu_cgs.c
index 1111111..2222222 100644
--- a/sys/dev/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/sys/dev/drm/amd/amdgpu/amdgpu_cgs.c
@@ -462,6 +462,15 @@ static int amdgpu_cgs_get_firmware_info(struct cgs_device *cgs_device,
 		hdr = (const struct smc_firmware_header_v1_0 *)	adev->pm.fw->data;
 		amdgpu_ucode_print_smc_hdr(&hdr->header);
 		adev->pm.fw_version = le32_to_cpu(hdr->header.ucode_version);
 		ucode_size = le32_to_cpu(hdr->header.ucode_size_bytes);
 		ucode_start_address = le32_to_cpu(hdr->ucode_start_addr);
+		/* DF-4774: ucode_array_offset_bytes/ucode_size_bytes are
+		 * self-declared by the firmware file; amdgpu_ucode_validate()
+		 * only checks datasize == size_bytes.  Bound the payload
+		 * window against the real blob before building the pointer
+		 * handed to the powerplay SMC upload loops. */
+		if (le32_to_cpu(hdr->header.ucode_array_offset_bytes) >
+		    adev->pm.fw->datasize ||
+		    ucode_size > adev->pm.fw->datasize -
+		    le32_to_cpu(hdr->header.ucode_array_offset_bytes)) {
+			DRM_ERROR("Invalid SMU firmware header: payload outside blob\n");
+			return -EINVAL;
+		}
 		src = (const uint8_t *)(adev->pm.fw->data +
 		       le32_to_cpu(hdr->header.ucode_array_offset_bytes));

 		info->version = adev->pm.fw_version;