Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Medium] Ineffective fix and misunderstanding of C pointer lifetime in `kirin_drm_crtc_init`. Pre-existing issues: - [High] NULL pointer dereference when shared IRQ triggers before CRTC initialization completes. - [High] Resource leaks and permanent driver breakage due to `devm_*` misuse in component `bind`. -- commit 5e60641f8f98c64d6ba73a1036397dc68df8729a Author: WenTao Liang <[email protected]> fix: drm/hisilicon: kirin_drm_crtc_init: premature of_node_put leaves crtc->port as dangling pointer This commit attempts to fix a dangling pointer by moving the of_node_put() call after the crtc->port assignment in kirin_drm_crtc_init(). > diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > index 8a11c2df5b886..cc453b29b22c3 100644 > --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c > @@ -53,8 +53,8 @@ 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: Medium] Does moving this of_node_put() actually change the pointer lifetime? Since assigning crtc->port = port only copies the address without incrementing the reference count, it seems the pointer is just as dangling as it was before. > > ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL, [Severity: High] This is a pre-existing issue, but can ade_irq_handler() trigger a NULL pointer dereference if a shared interrupt fires early? In drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c:ade_hw_ctx_alloc(), the IRQ is registered with IRQF_SHARED before the CRTC is initialized: ret = devm_request_irq(dev, ctx->irq, ade_irq_handler, IRQF_SHARED, dev->driver->name, ctx); At this point, crtc->dev is still NULL until drm_crtc_init_with_planes() is called later. If a shared interrupt fires or is left asserted by the bootloader, ade_irq_handler() calls drm_crtc_handle_vblank(crtc), which dereferences crtc->dev. [Severity: High] This is also a pre-existing issue, but does using dev-managed allocators inside a component bind callback cause resource leaks on unbind? In drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c:ade_hw_ctx_alloc(), which is called from the component bind path (kirin_drm_bind), resources are allocated using devm_platform_ioremap_resource() and other devm_* APIs. When the component is unbound, the master device is not removed, so devm does not release these resources. Rebinding will execute ade_hw_ctx_alloc() again, leaking memory, duplicating the IRQ handler, and causing ioremap to fail with -EBUSY. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
