[PATCH] drm: i915: selftests: fix oa_config refcount leak in test_stream()

[PATCH] drm: i915: selftests: fix oa_config refcount leak in test_stream()

[Wentao Liang]

test_stream() calls i915_perf_get_oa_config() which takes a reference
on the returned oa_config. On the error path where stream allocation
fails, the function returns without calling i915_oa_config_put() on
the oa_config, leaking the reference.

Move the oa_config check after the props.engine check and add
i915_oa_config_put(oa_config) on the error path to properly release
the reference.

Fixes: 9677a9f3b1ad ("drm/i915/perf: Move gt-specific data from i915->perf to gt->perf")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/i915/selftests/i915_perf.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_perf.c b/drivers/gpu/drm/i915/selftests/i915_perf.c
index e9469e27f42a..a3e0afb0549e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_perf.c
+++ b/drivers/gpu/drm/i915/selftests/i915_perf.c
@@ -104,14 +104,16 @@ test_stream(struct i915_perf *perf)
 	struct i915_perf_stream *stream;
 	struct intel_gt *gt;
 
-	if (!props.engine)
-		return NULL;
-
 	gt = props.engine->gt;
 
 	if (!oa_config)
 		return NULL;
 
+	if (!props.engine) {
+		i915_oa_config_put(oa_config);
+		return NULL;
+	}
+
 	props.metrics_set = oa_config->id;
 
 	stream = kzalloc_obj(*stream);
-- 
2.34.1

Claude review: drm: i915: selftests: fix oa_config refcount leak in test_stream()

[Claude Code Review Bot]

Overall Series Review

Subject: drm: i915: selftests: fix oa_config refcount leak in test_stream()
Author: Wentao Liang <vulab@iscas.ac.cn>
Patches: 1
Reviewed: 2026-05-28T12:42:28.615639

---

**NAK.** This single-patch series attempts to fix a refcount leak on the `!props.engine` error path in `test_stream()`, but the fix **introduces a NULL pointer dereference** that is strictly worse than the original leak. The patch moves the `!props.engine` check to *after* the line `gt = props.engine->gt;`, meaning if `props.engine` is NULL, the kernel will crash with a NULL dereference before it ever reaches the new guard.

The identified bug (leaking the `oa_config` reference when `props.engine` is NULL) is real, but the fix is incorrect.

---
Generated by Claude Code Patch Reviewer

Claude review: drm: i915: selftests: fix oa_config refcount leak in test_stream()

[Claude Code Review Bot]

Patch Review

**Bug: NULL pointer dereference introduced**

The original code correctly checked `props.engine` before dereferencing it:

```c
if (!props.engine)
    return NULL;

gt = props.engine->gt;
```

The patch removes that guard and moves it *after* the dereference:

```diff
-	if (!props.engine)
-		return NULL;
-
 	gt = props.engine->gt;
 
 	if (!oa_config)
 		return NULL;
 
+	if (!props.engine) {
+		i915_oa_config_put(oa_config);
+		return NULL;
+	}
```

If `props.engine` is NULL, line `gt = props.engine->gt;` will dereference a NULL pointer and crash before ever reaching the new `if (!props.engine)` check. This makes the new guard dead code and introduces a kernel oops on the path it claims to fix.

**The original bug is real but minor.** The commit message correctly identifies that when `props.engine` is non-NULL but `oa_config` is NULL, or when `props.engine` is NULL, there are paths that leak the `oa_config` refcount. However:

1. This is selftest code, not production driver code.
2. `props.engine` being NULL requires `intel_engine_lookup_user()` to fail for render engine class 0, which is unlikely on any system actually running i915 selftests.

**Correct fix approach.** The proper fix keeps the engine check first (before the dereference) and simply adds the refcount release:

```c
if (!props.engine) {
    i915_oa_config_put(oa_config);
    return NULL;
}

gt = props.engine->gt;

if (!oa_config)
    return NULL;
```

Note that `i915_oa_config_put()` already handles NULL (`i915_perf.h:54`), so no additional NULL check on `oa_config` is needed before calling it.

**Cc: stable is unwarranted.** This is a selftest-only refcount leak with no user-visible impact; backporting to stable is unnecessary.

**Fixes tag validity.** The referenced commit `9677a9f3b1ad` is plausible as the commit that restructured perf data, but the leak predates it — the original code never released `oa_config` on the `!props.engine` path regardless. This may need a different or additional Fixes tag.

---
Generated by Claude Code Patch Reviewer