Summary

In the new-format firmware path of cik_sdma_load_microcode(), the header fields ucode_array_offset_bytes and ucode_size_bytes are read from the firmware image and used to compute a pointer and loop bound without validating that offset + size stays within fw->datasize. The only validation performed upstream is radeon_ucode_validate() (radeon_ucode.c:156-165), which checks fw->datasize == hdr->size_bytes but does NOT validate the offset or payload-size fields. A corrupted or malicious firmware image with an oversized offset or size causes the ucode-loading loop to read past the end of the firmware allocation.

Root cause

At cik_sdma.c:471-472, the firmware data pointer rdev->sdma_fw->data is cast to sdma_firmware_header_v1_0*. At line 476, radeon_ucode_print_sdma_hdr dereferences SDMA-specific header fields (ucode_feature_version at offset 28, jt_offset at offset 36, etc.) without verifying the firmware is large enough to contain them.

At lines 479-480:

fw_data = (const __le32 *)(rdev->sdma_fw->data +
                            le32_to_cpu(hdr->header.ucode_array_offset_bytes));

The offset ucode_array_offset_bytes is a u32 taken directly from the firmware with no bounds check. At line 481: fw_size = le32_to_cpu(hdr->header.ucode_size_bytes) / 4 — also unbounded.

The loop at lines 483-484:

for (i = 0; i < fw_size; i++)
    WREG32(..., le32_to_cpup(fw_data++));

reads fw_size consecutive u32s starting at fw_data, which can extend past fw->datasize.

The validation at radeon_ucode.c:161 only checks fw->datasize == le32_to_cpu(hdr->size_bytes) — a self-declared size field that an attacker can set to match datasize while leaving ucode_array_offset_bytes or ucode_size_bytes pointing out of bounds.

Threat model

An attacker who can replace or corrupt the radeon SDMA firmware module (radeonkmsfw_<chip>_sdma) on disk can craft a firmware image where size_bytes matches the file length (passing radeon_ucode_validate) but ucode_array_offset_bytes or ucode_size_bytes point past the allocation. When the driver loads at init or resume, cik_sdma_load_microcode reads past the firmware buffer.

On DragonFlyBSD, firmware is loaded via firmware_get() from kernel modules (dev/drm/include/linux/firmware.h:40), requiring root filesystem write access or the ability to load a malicious kernel module.

Impact: kernel heap OOB read via le32_to_cpup, most likely causing a kernel panic (page fault on unmapped memory) — a local DoS requiring root to plant. No direct info leak to userspace: the read data is written only to GPU SDMA UCODE registers (WREG32), not returned to any user. This is primarily a defense-in-depth / robustness issue for corrupted firmware files (bit rot, incomplete writes).

PoC

Requires root to plant a crafted firmware module. Steps:

1. As root, craft a firmware file where the common_firmware_header has: size_bytes = <actual_file_size> (to pass radeon_ucode_validate), ucode_array_offset_bytes = 0xFFFFFF00 (points ~4 GB past data start), ucode_size_bytes = 0x1000.
2. Load it as the radeonkmsfw_<chip>_sdma firmware module (or replace the existing one in the firmware path).
3. Load/attach the radeon driver (kldload radeonkms or trigger device probe).
4. cik_sdma_load_microcode computes fw_data = rdev->sdma_fw->data + 0xFFFFFF00, then loops reading u32s from that address. This immediately page-faults on unmapped kernel memory.
5. Result: kernel panic (data abort / page fault). Success = immediate panic at le32_to_cpup in the firmware loading loop.

For a non-crash OOB variant: set ucode_array_offset_bytes to datasize - 4 and ucode_size_bytes to 0x1000. The first read is the last 4 bytes of the firmware (in bounds), subsequent reads go past the allocation into adjacent kernel heap (slightly OOB), potentially leaking heap data to GPU registers without crashing if the adjacent pages are mapped.

Recommended fix

Add bounds validation of ucode_array_offset_bytes and ucode_size_bytes against fw->datasize before using them. The validation belongs in cik_sdma_load_microcode (or preferably centralized in radeon_ucode_validate / a new helper).

--- a/sys/dev/drm/radeon/cik_sdma.c
+++ b/sys/dev/drm/radeon/cik_sdma.c
@@ -470,6 +470,7 @@ static int cik_sdma_load_microcode(struct radeon_device *rdev)
    if (rdev->new_fw) {
        const struct sdma_firmware_header_v1_0 *hdr =
            (const struct sdma_firmware_header_v1_0 *)rdev->sdma_fw->data;
+       const struct common_firmware_header *chdr = &hdr->header;
        const __le32 *fw_data;
        u32 fw_size;
        u32 ucode_offset = le32_to_cpu(chdr->ucode_array_offset_bytes);
@@ -476,6 +477,14 @@ static int cik_sdma_load_microcode(struct radeon_device *rdev)

        radeon_ucode_print_sdma_hdr(&hdr->header);

+       /* Validate firmware header fields against actual data size */
+       if (rdev->sdma_fw->datasize < sizeof(*hdr)) {
+           dev_err(rdev->dev, "ci_sdma: firmware too small for header\n");
+           return -EINVAL;
+       }
+       if (ucode_offset + fw_size * 4 > rdev->sdma_fw->datasize) {
+           dev_err(rdev->dev, "ci_sdma: firmware ucode offset/size out of bounds\n");
+           return -EINVAL;
+       }
        /* sdma0 */
        fw_data = (const __le32 *)
            (rdev->sdma_fw->data + ucode_offset);

Note: the sizeof(*hdr) check also protects radeon_ucode_print_sdma_hdr (line 476) from reading SDMA-specific header fields past a too-short firmware. The same bounds-check pattern should be applied to all other cik_*_load_microcode functions that parse new-format firmware headers (cik.c, si.c, etc.).

Related findings

Firmware header OOB family:

• DF-1538 (gmc_v7_0.c fw header OOB)
• DF-1553 (cik_sdma.c — originally noted but now elevated to a real finding)
• DF-1598 (sdma_v2_4.c fw header OOB)