DF-5224 — bios_get_image() bounds check uses wrap-prone offset + size arithmetic (hardening)

File: sys/dev/drm/amd/display/dc/bios/bios_parser_helper.c:40 (pass 2, GLM 5.3) Severity: Info (hardening) · Confidence: speculative (hazard is real at code level; no current caller can trigger it — proven below)

What

uint8_t *bios_get_image(struct dc_bios *bp, uint32_t offset, uint32_t size)
{
    if (bp->bios && offset + size < bp->bios_size)   /* <-- wrap-prone */
        return bp->bios + offset;
    else
        return NULL;
}

This 90-line helper file's only memory-safety-relevant statement is this bounds check. It is the security boundary for every GET_IMAGE(type, offset) dereference in the DC atombios parsers (~60 sites in bios_parser.c + bios_parser2.c, plus the direct call at bios_parser.c:2022). The addition offset + size is performed in wrapping 32-bit arithmetic: for offset = 0xFFFFFFF2, size = 16 the sum wraps to 2 < bios_size and the function returns bp->bios + 0xFFFFFFF2 — a wild kernel pointer that callers immediately cast to an atom-table struct and dereference.

Why it is NOT reachable today (killed, with evidence)

Every offset expression reaching this function is bounded by two little-endian uint16_t VBIOS fields summed, plus small constants:

• data-table offsets: bp->master_data_tbl_offset + tbl->list.X.data_offset — both sides uint16 (atomfirmware.h:381-424, atom_master_list_of_data_tables_v2_1 is all uint16_t)
• object/record offsets: bp->object_info_tbl_offset + le16(field) (bios_parser.c:137,164,234,297,887,1527,1988,2015)
• ROM bootstrap offsets: fixed constants (OFFSET_TO_ATOM_ROM_HEADER_POINTER = 0x48, atomfirmware.h:210) or uint16_t image reads (bios_parser.c:2884-2923)

So offset ≤ 0xFFFF + 0xFFFF ≈ 0x1FFFE. Sizes are sizeof(struct) (largest atom struct here ~4 KB) or, for the one direct call (bios_parser.c:2022), *number * sizeof(uint16_t) where number is a uint8_t image byte (max 255, product computed in 64-bit via sizeof, max 510). Worst case offset + size ≈ 0x21000 — cannot wrap uint32_t. bios_size itself is bios[2] * 512 ≤ 130560 (bios_parser.c:2878).

The < comparator is conservative-correct (admits only strictly-interior windows), and bp->bios is NULL-guarded.

NOT a duplicate of DF-4168

DF-4168 (Medium, bios_parser2.c:1930) covers the reference point being wrong: bios_size is derived from the image's self-declared size byte (bios[2]*512) instead of the allocated buffer length, so even non-wrapping offsets can read past the real allocation. DF-5224 is about the check's own arithmetic: with DF-4168 fixed (honest bios_size), the wrap-prone form still silently admits offset near UINT32_MAX from any future/wrong caller. The two compose; they are distinct defects at distinct sites. (Also distinct from DF-3942/DF-1299, which are missing NULL/limit checks in the callers, not in this helper.)

Fix (hardening, upstream-portable)

See fix.diff — rewrite in overflow-safe form. Behavior is byte-identical for every in-range call that passes today; only wrap-candidate inputs change from "wild pointer" to "NULL".

Reproduce

Not runtime-reproducible on the audit guest: the trigger requires an AMD ASIC whose VBIOS content produces a 32-bit offset near UINT32_MAX, and no current parser expression can emit such an offset. Verified at source level only; that is appropriate for an Info hardening finding per the audit contract. Run verify.sh for the mechanical caller-bounds audit backing the negative reachability claim.

DF-5224 — VERDICT

Status: untested (Info hardening — source-level verification only, per audit contract for Low/Info findings). Reproduced: no. Impact: none (latent).

What I verified, line by line

bios_parser_helper.c is compiled into the live amdgpu.ko module (sys/dev/drm/amd/amdgpu/Makefile:208, .PATH includes ../display/dc/bios; CONFIG_DRM_AMD_DC 1 in sys/dev/drm/kconfig.h:23). It is reached at GPU attach; its BIOS image is adev->mode_info.atom_context->bios (amdgpu_dm.c:410-411 → dc.c:639 → bios_parser.c:2877 / bios_parser2.c:1929), i.e. firmware/hardware-supplied data — the attacker model for this whole file family is a crafted or corrupt VBIOS on the card (same precondition class as DF-5005/DF-5163/DF-5164/DF-4168).

The helper's check itself (bios_parser_helper.c:40)

• offset + size wraps mod 2^32 for offset ≥ 2^32 - size. With a wrapped sum below bios_size, the function returns bp->bios + offset — a kernel pointer ~4 GB away from the image buffer that callers immediately dereference as an atom table struct. The arithmetic hazard is real.
• Reachability: killed. All ~60 GET_IMAGE sites plus the one direct call feed offsets that are the sum of at most two uint16_t LE VBIOS fields (+ small constants): max ≈ 0x1FFFE. Sizes: sizeof(struct) ≤ ~4 KB, or ≤ 510 (64-bit-computed, bios_parser.c:2022). Max sum ≈ 0x21000 ≪ 2^32. No wrap is producible by any caller in this tree. (Mechanical audit of every call site: see README.md and verify.sh output in run.log.)
• < (vs <=) is the conservative direction — safe.
• bp->bios NULL-guarded (:40).
• bios_size provenance is DF-4168's finding (bios_parser2.c:1930, bios_parser.c:2878: content-derived bios[2]*512 vs real allocation); explicitly not re-reported here — this finding is only about the check's arithmetic and is meaningful after DF-4168 is fixed.

The rest of the file (hunt items a–e, all clean)

• (a) GET_* helpers / index bounds / casts: bios_get_image is the only image helper; GET_IMAGE macro (bios_parser_helper.h:39) adds nothing. No index arithmetic lives in this file.
• (b) shifts/masks/sign: FN (:54-55) expands to ATOM_S6_ACC_MODE_SHIFT=4 / ATOM_S6_ACC_MODE=0x10 and ATOM_S6_CRITICAL_STATE_SHIFT=8 / ATOM_S6_CRITICAL_STATE=0x100 (atombios.h:6959,6963,7031,7035) — mask == 1<<shift for both, coherent pairs, single-bit fields, unsigned throughout. No sign confusion.
• (c) callers: both parsers (bios_parser.c v1 path via dal_bios_parser_create fallback, bios_parser2.c firmware-parser path, bios_parser_interface.c:41-48) — firmware-table-driven, as expected.
• (d) build/users: amdgpu.ko only; bios_is_accelerated_mode, bios_set_scratch_*, bios_get_vga_enabled_displays consumed by DC init/HPD paths.
• (e) register helpers: bios->regs->BIOS_SCRATCH_6 is defined non-zero on every constructible DC: dce80/100/110/112 (*_resource.c: .BIOS_SCRATCH_6 = 0x05CF), dce120/dcn10 (NBIO segment macros). So the unguarded REG_* SCRATCH_6 uses are safe on all shipped ASICs. The if (bios->regs->BIOS_SCRATCH_3) guard at :86 exists precisely because pre-SOC15 bios_regs structs only populate SCRATCH_6 — deliberate, and 0xFFFF masking of the SCRATCH_3 count matches the 16-bit field.
• Command-table struct consistency: the file includes command_table.h but consumes nothing from it; no struct skew possible from this file.

Fix

fix.diff (authored post-verification, never applied to sys/):

-   if (bp->bios && offset + size < bp->bios_size)
+   if (bp->bios && offset < bp->bios_size &&
+           size <= bp->bios_size - offset)

Overflow-safe, behavior-identical for all honest inputs, converts wrap-candidate inputs into NULL (which callers such as DF-3942's NULL-deref sites already expect — the caller-side NULL-check gap is separately tracked by DF-3942 and is unaffected in the safe direction).

Why no QEMU run

The audit guest has no AMD GPU and no path by which an unprivileged local user can supply a VBIOS; the finding's trigger set is empty on this tree (proven above). A runtime "verification" would be theater. verify.sh reruns the mechanical caller-bounds audit so the negative claim is reproducible by anyone.

Bottom line

File the Info hardening note: the DC's single image-bounds check is written in wrap-prone arithmetic; fix it while fixing the DF-4168 family so the eventually-honest bios_size is checked with eventually-honest arithmetic. No exploitable defect exists in this file today.

Fix status (batch validation)

Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest make -j6 nativekernel KERNCONF=X86_64_GENERIC completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.