Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The patch attempts to fix a dangling pointer (use-after-free) by simply swapping the order of a pointer assignment (`crtc->port = port`) and the reference release (`of_node_put(port)`). This fix is completely ineffective. In C, assigning a pointer does not increment the reference count. `of_node_put(port)` still drops the reference acquired by `of_get_child_by_name()`. When `kirin_drm_crtc_init()` returns, the CRTC holds a raw pointer to a device node without holding an active reference to it, leaving it exactly as vulnerable as before. -- commit a7a4845c1da7436e4f1b2446968b4b040228f36f Author: WenTao Liang <[email protected]> drm/hisilicon: fix dangling pointer in kirin_drm_crtc_init This patch attempts to fix a dangling pointer by moving the of_node_put() call after assigning the port to crtc->port. > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > index 8a11c2df5b886..2fda894d482e4 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c [ ... ] > @@ -53,8 +53,9 @@ static int kirin_drm_crtc_init(struct drm_device *dev, > struct drm_crtc *crtc, > DRM_ERROR("no port node found in %pOF\n", dev->dev->of_node); > return -EINVAL; > } > - of_node_put(port); > + > crtc->port = port; > + of_node_put(port); [Severity: High] This isn't a bug introduced by this patch, but does this actually fix the dangling pointer issue? Since pointer assignment does not increment the reference count, dropping the reference with of_node_put() here leaves crtc->port pointing to a device node without holding an active reference to it. If the device node is subsequently removed (for example, via device tree overlay removal), couldn't this still lead to a use-after-free when the DRM subsystem later accesses crtc->port? To properly resolve the dangling pointer, should the reference be held for the lifetime of the CRTC and released in the CRTC destruction path instead of here in kirin_drm_crtc_init()? > > ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL, > driver_data->crtc_funcs, NULL); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
