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/kmb: Fix error pointer dereference
Date: Mon, 23 Feb 2026 06:12:42 +1000
Message-ID: <review-patch1-20260219224426.1122073-1-ethantidmore06@gmail.com>
In-Reply-To: <20260219224426.1122073-1-ethantidmore06@gmail.com>
References: <20260219224426.1122073-1-ethantidmore06@gmail.com> <20260219224426.1122073-1-ethantidmore06@gmail.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

The bug analysis in the commit message is accurate. Confirmed that `kmb_dsi_host_unregister()` immediately dereferences its argument:

```c
void kmb_dsi_host_unregister(struct kmb_dsi *kmb_dsi)
{
	kmb_dsi_clk_disable(kmb_dsi);
	mipi_dsi_host_unregister(kmb_dsi->host);
}
```

So passing an ERR_PTR here will crash.

There are two paths that reach `err_free1`:

1. `kmb_dsi_init()` fails -- `kmb->kmb_dsi` is an ERR_PTR, must not call unregister.
2. `kmb_hw_init()` fails -- `kmb->kmb_dsi` is a valid pointer, must call unregister.

The fix handles both correctly:

> +	if (!IS_ERR(kmb->kmb_dsi))
> +		kmb_dsi_host_unregister(kmb->kmb_dsi);

This works, though the more common kernel pattern for this situation is to add a separate goto label so the `kmb_dsi_init` failure path skips the unregister entirely, rather than adding a runtime check in the cleanup code. Something like having the `kmb_dsi_init` error path jump to a label after the unregister call (or before it, with `dev_set_drvdata` duplicated). The IS_ERR approach is functionally fine but slightly unusual for kernel error unwinding, where the convention is that each label corresponds to unwinding one specific resource.

That said, restructuring the labels would be a larger diff for minimal benefit. The fix as-is is correct and solves the bug.

No other issues found.

---
Generated by Claude Code Patch Reviewer