[PATCH] drm/amdkfd: stop speculation on the kfd_ioctl path

[PATCH] drm/amdkfd: stop speculation on the kfd_ioctl path

[Greg Kroah-Hartman]

The kfd_ioctl takes a user controlled pointer, and then dereferences it
into a table of function pointers, the signature method of spectre
problems.  Fix this up by calling array_index_nospec() on the index to
the function pointer list.

Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 09dabb3b3297..d2ef693c63da 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -35,6 +35,7 @@
 #include <linux/mman.h>
 #include <linux/ptrace.h>
 #include <linux/dma-buf.h>
+#include <linux/nospec.h>
 #include <linux/processor.h>
 #include "kfd_priv.h"
 #include "kfd_device_queue_manager.h"
@@ -3349,6 +3350,7 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 	if ((nr >= AMDKFD_COMMAND_START) && (nr < AMDKFD_COMMAND_END)) {
 		u32 amdkfd_size;
 
+		nr = array_index_nospec(nr, AMDKFD_CORE_IOCTL_COUNT);
 		ioctl = &amdkfd_ioctls[nr];
 
 		amdkfd_size = _IOC_SIZE(ioctl->cmd);
-- 
2.53.0

Claude review: drm/amdkfd: stop speculation on the kfd_ioctl path

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amdkfd: stop speculation on the kfd_ioctl path
Author: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Patches: 1
Reviewed: 2026-03-25T06:37:21.620119

---

This is a single-patch series from Greg Kroah-Hartman adding a Spectre v1 mitigation to the KFD ioctl dispatch path. The intent is correct — `nr` is derived from user-controlled `cmd` via `_IOC_NR()`, and is used to index into the `amdkfd_ioctls[]` function pointer table. This is a classic Spectre v1 gadget. The fix uses `array_index_nospec()` to clamp the index under speculation.

The patch is **correct and appropriate**, though the placement of the barrier could be slightly improved. Overall this is a reasonable hardening patch suitable for stable backport.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amdkfd: stop speculation on the kfd_ioctl path

[Claude Code Review Bot]

Patch Review

**Correctness: Good, with one minor observation**

The bounds check at line 3345 (`nr >= AMDKFD_CORE_IOCTL_COUNT`) provides the architectural bounds check, and the `array_index_nospec()` at line 3353 provides the speculation barrier. This is the correct pattern.

However, there's a subtle point worth noting. The bounds check at line 3345:

```c
if (nr >= AMDKFD_CORE_IOCTL_COUNT) {
    retcode = -ENOTTY;
    goto err_i1;
}
```

checks against `AMDKFD_CORE_IOCTL_COUNT` (the full array size), while the subsequent check at line 3350:

```c
if ((nr >= AMDKFD_COMMAND_START) && (nr < AMDKFD_COMMAND_END)) {
```

further narrows the valid range to `[AMDKFD_COMMAND_START, AMDKFD_COMMAND_END)` i.e. `[0x01, 0x28)`. The `array_index_nospec(nr, AMDKFD_CORE_IOCTL_COUNT)` at line 3353 clamps to the full array size, which is correct since the array is indexed by ioctl number using designated initializers (`[_IOC_NR(ioctl)] = ...`). Using `AMDKFD_CORE_IOCTL_COUNT` as the bound is the right choice here — it matches the actual array size, and out-of-range entries within the array simply have `.func = NULL` which is handled later.

**Placement consideration:** The `array_index_nospec()` call is placed inside the `if ((nr >= AMDKFD_COMMAND_START) && (nr < AMDKFD_COMMAND_END))` block. This is fine because the only array dereference (`&amdkfd_ioctls[nr]`) occurs within this block. The earlier bounds check at line 3345 ensures the architectural path is safe, and the nospec call handles the speculative path.

**Include:** The `<linux/nospec.h>` addition is correctly placed in alphabetical order.

**Cc: stable** tag is appropriate — this is a security hardening fix.

**Nit:** The `Assisted-by: gkh_clanker_2000` tag is unconventional. Typically AI/tool assistance is noted with a more descriptive tag or in the commit message body, but this is a process/policy matter, not a code issue.

**Verdict: Patch looks good.** No functional issues.

---
Generated by Claude Code Patch Reviewer