DF-1300 — dc_add_all_planes_for_stream unchecked plane_count stack overflow (dead API)

Severity: Medium · CWE: CWE-787 (Out-of-bounds Write) File: sys/dev/drm/amd/display/dc/core/dc_resource.c:1564-1571

Build & run (AMD-DC latent bug, DEAD API — no AMD GPU on guest, harness proof)

./build.sh   # cc -O0 -Wall -o harness harness.c   (-O0 required)
./run.sh     # ./harness

Expected output (bug present)

saved_rbp_canary = 0x00007fffffdfd910  *** CORRUPTED ***
saved_rip_canary = 0x00007fffffdfd914  *** CORRUPTED ***
RESULT: stack buffer overflow CONFIRMED at dc_resource.c:1570-1571

Why -O0

At -O2 gcc's -faggressive-loop-optimizations may silently clamp the loop to MAX_SURFACES iterations (the OOB writes are UB and get elided). The kernel builds at -O2 and MAY compile out the bug — but that is fragile and the correct fix is the explicit bounds check in fix.diff.

Status

Dead public API (0 in-tree callers). Latent stack-overflow hazard for any future DC consumer. See VERDICT.md.

DF-1300 — dc_add_all_planes_for_stream unchecked plane_count stack overflow (dead API)

Verdict: REPRODUCED (source-level + harness) — latent AMD-DC bug, stack overflow (dead API)

The AMD Display Core (DC) dc_resource.c is part of the amdgpu DRM module, not in X86_64_GENERIC and no AMD GPU is present on the audit guest. Moreover dc_add_all_planes_for_stream is a dead public API — zero in-tree callers (only the definition at dc_resource.c:1557 and the prototype at dc_stream.h:218). The bug is therefore a latent hazard: confirmed by source trace and reproduced at the object level, but not triggerable on this guest. Any future DC consumer that wires this API without clamping plane_count to MAX_SURFACES gets a stack overflow.

The bug

sys/dev/drm/amd/display/dc/core/dc_resource.c:1557-1574:

bool dc_add_all_planes_for_stream(..., int plane_count, ...)
{
    struct dc_validation_set set;       /* ON THE STACK */
    int i;
    set.stream = stream;
    set.plane_count = plane_count;      /* :1568 raw caller, NO clamp */
    for (i = 0; i < plane_count; i++)   /* :1570 NO check vs MAX_SURFACES */
        set.plane_states[i] = plane_states[i];   /* :1571 the overflow */
    ...
}

struct dc_validation_set (dc.h:597-601):

#define MAX_SURFACES 3                      /* dc.h:43 */
struct dc_validation_set {
    struct dc_stream_state *stream;         /* off 0  */
    struct dc_plane_state  *plane_states[MAX_SURFACES];  /* off 8, 3 ptrs */
    uint8_t plane_count;                    /* off 32 (immediately after) */
};

plane_states[3] (the 4th element) aliases the plane_count field; plane_states[4+] aliases stack padding, the saved frame pointer, and the return address. A plane_count > MAX_SURFACES is a classic stack buffer overflow that, depending on the caller-controlled pointer values, can hijack control flow on return.

Dead-API verification

$ grep -rn "dc_add_all_planes_for_stream" sys/dev/drm/
sys/dev/drm/amd/display/dc/core/dc_resource.c:1557:bool dc_add_all_planes_for_stream(   # definition
sys/dev/drm/amd/display/dc/dc_stream.h:218:bool dc_add_all_planes_for_stream(          # prototype

Zero call sites. The function is exported as public DC API, so an out-of-tree or future in-tree consumer could call it.

Harness proof

harness.c replicates dc_validation_set layout with canary fields after plane_states[] (standing in for plane_count + saved registers), calls the replicated function with plane_count=8, and shows the writes past plane_states[2] corrupt the canaries with attacker-supplied stack pointers. Decisive output:

MAX_SURFACES=3  sizeof(plane_states)=24 (3 ptrs)
calling vulnerable_add_planes(plane_count=8) -- MAX_SURFACES=3
=> 5 OOB writes past plane_states[MAX_SURFACES-1] expected
BEFORE: plane_count=0  saved_rbp_canary=0xdeadbeefcafebabe  saved_rip_canary=0x1122334455667788
AFTER:
  plane_states[0..2] = 0x7fffffdfd900..08  (in-bounds)
  plane_count (overwritten by [3]) = 12
  saved_rbp_canary = 0x00007fffffdfd910  *** CORRUPTED ***
  saved_rip_canary = 0x00007fffffdfd914  *** CORRUPTED ***
RESULT: stack buffer overflow CONFIRMED at dc_resource.c:1570-1571

-O0 is required: at -O2 gcc's -faggressive-loop-optimizations sees the plane_states[MAX_SURFACES] array bound and may silently clamp the loop to 3 iterations (declaring the OOB writes UB and eliding them). This optimizer behavior is itself worth noting — it means the bug may be compiled out of some kernels, but relying on that is fragile (gcc version/flag dependent) and the correct fix is the explicit bounds check. The kernel's actual behavior depends on the gcc version and CFLAGS used to build amdgpu.ko.

Build & run

./build.sh   # cc -O0 -Wall -o harness harness.c   (-O0 required, see above)
./run.sh     # ./harness

Fix

fix.diff clamps plane_count to MAX_SURFACES before the loop:

if (plane_count > MAX_SURFACES)
    plane_count = MAX_SURFACES;

This matches the finding proposal ("check plane_count<=MAX_SURFACES") and makes the dead API safe for any future caller. (Clamping rather than returning false preserves the current contract of "add up to MAX_SURFACES planes".)