From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/tegra: rgb: Fix use-after-free and leak in tegra_dc_rgb_probe() Date: Sun, 12 Apr 2026 13:58:42 +1000 Message-ID: In-Reply-To: <20260407084629.283151-1-vulab@iscas.ac.cn> References: <20260407084629.283151-1-vulab@iscas.ac.cn> <20260407084629.283151-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 **Claimed bug 1 =E2=80=94 "use-after-free":** The commit message states the= reorder avoids "using np after it may have been freed (if the action regis= tration fails)." This is incorrect. In the original code: ```c err =3D devm_add_action_or_reset(dc->dev, tegra_dc_of_node_put, np); if (err < 0) return err; if (!of_device_is_available(np)) return -ENODEV; ``` If `devm_add_action_or_reset()` fails, the `_or_reset` part calls `tegra_dc= _of_node_put(np)` (freeing `np`), and then `err < 0` causes an **immediate = return**. The `of_device_is_available(np)` call is never reached. There is = no use-after-free. **Claimed bug 2 =E2=80=94 "reference leak":** The commit message claims `np= ` leaks "when the device is not available." This is also incorrect. In the = original code, when `of_device_is_available(np)` returns false, `tegra_dc_r= gb_probe()` returns `-ENODEV`. The caller in `dc.c:3255` explicitly ignores= `-ENODEV`: ```c err =3D tegra_dc_rgb_probe(dc); if (err < 0 && err !=3D -ENODEV) return dev_err_probe(&pdev->dev, err, ...); ``` The overall device probe **succeeds**, so the devm action registered by `de= vm_add_action_or_reset()` remains active and will correctly call `of_node_p= ut(np)` when the device is eventually removed. There is no leak. **Assessment:** The patch is a **no-op from a correctness standpoint** =E2= =80=94 the reordering produces functionally equivalent behavior. The code a= fter the patch is marginally tidier (avoids leaving a devm action registere= d for a node that won't be used), but this is a minor cleanup, not a bug fi= x. The `Fixes:` tag and `Cc: stable` are unwarranted. **Recommendation:** NAK in its current form. If the author wants to submit = this as a minor cleanup/style improvement without the `Fixes:` tag, `Cc: st= able`, and the incorrect bug claims in the commit message, that would be a = more honest framing =E2=80=94 though even then, the value is marginal. --- Generated by Claude Code Patch Reviewer