amdgpu_atombios_i2c_channel_trans() omits the memset of its stack atom parameter block that its sibling performs — defense-in-depth only, every struct byte is coincidentally initialized today
Summary
The gfx_v8_0 init path assigns 7 fields but skips the sibling's memset - fully covered today (8 bytes, no padding) but any struct growth or crafted-VBIOS PS sizing silently exposes kernel stack to the atom interpreter. Add-the-memset fix.
Discussion (0)
PoC verification
Evidence pack
findings/poc/DF-5164 · 4 files| File | Type | Description | Size | |
|---|---|---|---|---|
| fix.diff | — | 569 B | view raw | |
| README.md | — | 1.6 KB | ↓ raw | |
| manifest.json | — | 776 B | view raw | |
| verdict.json | — | 2.1 KB | view raw |
DF-5164 — amdgpu_atombios_i2c_channel_trans() skips the parameter-block memset its sibling performs
PoC verdict: untested (Info hardening; explicitly out of PoC scope per
audit contract). No runtime impact demonstrated — see VERDICT.md.
What and why
amdgpu_atombios_i2c_process_i2c_ch() zeroes its stack parameter block
before handing it to the atom interpreter:
PROCESS_I2C_CHANNEL_TRANSACTION_PS_ALLOCATION args; /* atombios_i2c.c:44 */
...
memset(&args, 0, sizeof(args)); /* atombios_i2c.c:50 */
amdgpu_atombios_i2c_channel_trans() (atombios_i2c.c:167-181, called from
sys/dev/drm/amd/amdgpu/gfx_v8_0.c:795-796 for Polaris10 golden-register
I2C writes at driver init) declares the same struct and assigns seven of its
fields — which today happens to cover every byte of the 8-byte struct
(ucI2CSpeed0, ucRegIndex1, lpI2CDataOut2-3, ucFlag4, ucTransBytes5,
ucSlaveAddr6, ucLineNumber7 — sys/dev/drm/amd/include/atombios.h:8477-8492).
There is no padding, so no uninitialized byte currently reaches the
interpreter. The finding is the missing defense-in-depth only: any future
field addition, struct version bump (the _PS_ALLOCATION macros in other
tables are unions with larger firmware workspaces), or VBIOS table whose
declared parameter-space size exceeds 8 bytes (the interpreter trusts the
table header — atom.c:1211) would silently turn this into an
uninitialized-stack disclosure into the atom execution path.
Fix
fix.diff — one memset(&args, 0, sizeof(args)); matching the sibling.
Fix verification
not_testableNo behavioral delta to validate: the finding has no demonstrated runtime impact today; the one-line fix is compile-level and applies cleanly.
fix.diff
Confirmed kernel references
Detail
Evidence (decisive lines)
README.md (field-by-field proof that all 8 bytes are currently initialized); fix.diff (one-line memset, git apply --check clean)
PoC changes
not applicable — no PoC (Info hardening finding, explicitly skipped per contract)
Verified recommended fix
memset(&args, 0, sizeof(args)) in amdgpu_atombios_i2c_channel_trans before field assignment (see fix.diff).
Verdict
Info hardening, verified at source level only (no runtime test, per contract, for Low/Info findings): amdgpu_atombios_i2c_channel_trans() (atombios_i2c.c:167-181) passes its stack PROCESS_I2C_CHANNEL_TRANSACTION_PS_ALLOCATION to the atom interpreter without the memset its sibling performs at atombios_i2c.c:50. Today all 8 struct bytes are explicitly assigned (struct at sys/dev/drm/amd/include/atombios.h:8477-8492 has no padding), so no uninitialized byte currently reaches firmware; the omission only becomes live if the struct grows or a VBIOS table declares a larger parameter space (interpreter trusts the table header, atom.c:1209-1211).
No comments yet.