From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm: i915: selftests: fix oa_config refcount leak in test_stream()
Date: Thu, 28 May 2026 12:42:29 +1000
Message-ID: <review-patch1-20260527040835.854065-1-vulab@iscas.ac.cn>
In-Reply-To: <20260527040835.854065-1-vulab@iscas.ac.cn>
References: <20260527040835.854065-1-vulab@iscas.ac.cn> <20260527040835.854065-1-vulab@iscas.ac.cn>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Bug: NULL pointer dereference introduced**

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

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

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

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

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

If `props.engine` is NULL, line `gt =3D 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 identi=
fies 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 (befo=
re the dereference) and simply adds the refcount release:

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

gt =3D props.engine->gt;

if (!oa_config)
    return NULL;
```

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

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

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

---
Generated by Claude Code Patch Reviewer