* [PATCH] drm/radeon: Fix OOB read in MC register table init
@ 2026-05-29 12:14 Nikita Zhandarovich
2026-06-04 6:32 ` Claude review: " Claude Code Review Bot
2026-06-04 6:32 ` Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Nikita Zhandarovich @ 2026-05-29 12:14 UTC (permalink / raw)
To: Alex Deucher
Cc: Nikita Zhandarovich, Christian König, David Airlie,
Simona Vetter, amd-gfx, dri-devel, linux-kernel, lvc-project
radeon_atom_init_mc_reg_table() copies the previous mc_data[] entry when
pre_reg_data requests DATA_EQU_PREV. However, the loop starts at i == 0,
so a malformed or unexpected table can make the first entry use
DATA_EQU_PREV and trigger an out-of-bounds read from mc_data[i - 1].
Emulate a fix for a similar issue in amdgpu_atombios_init_mc_reg_table,
see commit 51dfc0a4d609 ("drm/amdgpu: fix mc_data out-of-bounds read
warning"), by skipping DATA_EQU_PREV for the first entry.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: ae5b0abbb6f7 ("drm/radeon/kms: add atom helper functions for dpm (v3)")
Cc: stable@vger.kernel.org
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
---
P.S. checkpatch warns that too many tabs were used but I can't do much
about surrounding code being already too deeply nested.
drivers/gpu/drm/radeon/radeon_atombios.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c b/drivers/gpu/drm/radeon/radeon_atombios.c
index 3dd9724b331d..c6d229a4322f 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -4032,6 +4032,8 @@ int radeon_atom_init_mc_reg_table(struct radeon_device *rdev,
(u32)le32_to_cpu(*((u32 *)reg_data + j));
j++;
} else if ((reg_table->mc_reg_address[i].pre_reg_data & LOW_NIBBLE_MASK) == DATA_EQU_PREV) {
+ if (i == 0)
+ continue;
reg_table->mc_reg_table_entry[num_ranges].mc_data[i] =
reg_table->mc_reg_table_entry[num_ranges].mc_data[i - 1];
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Claude review: drm/radeon: Fix OOB read in MC register table init
2026-05-29 12:14 [PATCH] drm/radeon: Fix OOB read in MC register table init Nikita Zhandarovich
@ 2026-06-04 6:32 ` Claude Code Review Bot
2026-06-04 6:32 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 6:32 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/radeon: Fix OOB read in MC register table init
Author: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Patches: 1
Reviewed: 2026-06-04T16:32:17.369199
---
This is a single, straightforward defensive fix for a potential out-of-bounds read in the radeon driver's ATOMBIOS MC register table initialization. The patch is correct, minimal, and mirrors an identical fix already applied to the amdgpu counterpart (commit 51dfc0a4d609). The bug is real: when `i == 0` and `DATA_EQU_PREV` is set, `mc_data[i - 1]` reads `mc_data[-1]`, which is an OOB access into the preceding struct member or adjacent memory.
**Verdict: Looks good.** No concerns with correctness.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: drm/radeon: Fix OOB read in MC register table init
2026-05-29 12:14 [PATCH] drm/radeon: Fix OOB read in MC register table init Nikita Zhandarovich
2026-06-04 6:32 ` Claude review: " Claude Code Review Bot
@ 2026-06-04 6:32 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-06-04 6:32 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Problem analysis:** In `radeon_atom_init_mc_reg_table()`, the inner `for` loop at line 4029 iterates `i` starting from 0. When `pre_reg_data` has `DATA_EQU_PREV` set for the first entry (`i == 0`), the code accesses `mc_data[i - 1]` which underflows to `mc_data[-1]` — a clear out-of-bounds read.
**Fix analysis:** The patch adds:
```c
if (i == 0)
continue;
```
at line 4035-4036, before the `mc_data[i - 1]` access. This is correct:
1. **`continue` targets the right loop.** The innermost enclosing loop is the `for (i = 0, j = 1; i < reg_table->last; i++)` at line 4029. The `continue` skips the copy for entry 0 and advances `i`, which is the desired behavior — leaving `mc_data[0]` uninitialized for this case is acceptable since `DATA_EQU_PREV` on the first entry is a malformed table condition with no meaningful "previous" value to copy.
2. **Exact match to the amdgpu fix.** The amdgpu equivalent at `amdgpu_atombios.c:1525-1526` uses the identical pattern (`if (i == 0) continue;`), so this is consistent across both drivers.
3. **No side effects on `j`.** The `j` index is only incremented in the `DATA_FROM_TABLE` branch, not the `DATA_EQU_PREV` branch, so skipping via `continue` doesn't desynchronize the data pointer.
4. **Fixes tag and stable tag are appropriate.** The Fixes tag points to the original commit that introduced this code path (`ae5b0abbb6f7`), and the `Cc: stable` is warranted since this is a real OOB read.
**Minor note:** The commit message mentions "Emulate a fix" — the wording is slightly odd ("apply the same fix as" would be more natural), but this is cosmetic and not worth a respin.
**Reviewed-by worthy.** No issues found.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-04 6:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-29 12:14 [PATCH] drm/radeon: Fix OOB read in MC register table init Nikita Zhandarovich
2026-06-04 6:32 ` Claude review: " Claude Code Review Bot
2026-06-04 6:32 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox