# 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`:

```c
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`):

```c
#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:

```c
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".)