Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues: - [High] NULL pointer dereference in error path if devm_kzalloc() fails to allocate priv - [High] Use-After-Free due to DRM objects allocated with devm_kzalloc instead of drmm_kzalloc - [High] Kernel panic/Resource leak on driver unload if initialized without component framework - [High] Use-After-Free of devm-allocated bridges due to leakage in the global bridge_list - [High] Use-After-Free due to drm_dev_put() dropping the DRM device before component_unbind_all() accesses it -- --- Patch [1]: [PATCH v2] drm/meson: clean up KMS polling on register failure --- commit 86a9a25dd06d99107bc269bc3d6752c0df753219 Author: Myeonghun Pak <[email protected]> drm/meson: clean up KMS polling on register failure This commit ensures that drm_kms_helper_poll_fini() is called when drm_dev_register() fails in meson_drv_bind_master(). This stops the KMS polling helper before the probe error path unwinds the IRQ and frees the DRM device. Link: https://lore.kernel.org/r/[email protected] > diff --git a/drivers/gpu/drm/meson/meson_drv.c > b/drivers/gpu/drm/meson/meson_drv.c > index 49ff9f1f16d32..bcc36f2cdc9a9 100644 > --- a/drivers/gpu/drm/meson/meson_drv.c > +++ b/drivers/gpu/drm/meson/meson_drv.c > @@ -351,8 +351,10 @@ static int meson_drv_bind_master(struct device *dev, > bool has_components) > platform_set_drvdata(pdev, priv); > > ret = drm_dev_register(drm, 0); > - if (ret) > + if (ret) { > + drm_kms_helper_poll_fini(drm); > goto uninstall_irq; > + } > > drm_client_setup(drm, NULL); > [Severity: High] This isn't a bug introduced by this patch, but can this error path in meson_drv_bind_master() trigger a NULL pointer dereference? If devm_kzalloc() fails to allocate memory for priv, it jumps to free_drm: drivers/gpu/drm/meson/meson_drv.c:meson_drv_bind_master() { ... priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); if (!priv) { ret = -ENOMEM; goto free_drm; } ... } At the free_drm label, meson_encoder_dsi_remove(priv) is called, which immediately dereferences the NULL priv pointer: drivers/gpu/drm/meson/meson_encoder_dsi.c:meson_encoder_dsi_remove() { ... if (priv->encoders[MESON_ENC_DSI]) { ... } [Severity: High] This is a pre-existing issue, but does using devm_kzalloc() for the DRM plane risk a use-after-free? drivers/gpu/drm/meson/meson_plane.c:meson_plane_create() { ... meson_plane = devm_kzalloc(priv->drm->dev, sizeof(*meson_plane), GFP_KERNEL); ... } Because devm_kzalloc() ties the memory lifetime to the platform device, if the device is unbound (e.g., via sysfs) while userspace still holds a DRM file descriptor open, devres frees the plane structure. Later, when the file descriptor is closed, the DRM core cleans up and accesses this already-freed structure to invoke its .destroy hook. [Severity: High] This isn't a bug introduced by this patch, but does the driver unload sequence leak resources if initialized without the component framework? drivers/gpu/drm/meson/meson_drv.c:meson_drv_probe() { ... if (count && !match) return meson_drv_bind_master(&pdev->dev, false); ... } If no matching components are found, it bypasses the component framework. However, during driver teardown, it only cleans up via the component framework: drivers/gpu/drm/meson/meson_drv.c:meson_drv_remove() { ... component_master_del(&pdev->dev, &meson_drv_master_ops); } Because it was never added as a component master, meson_drv_unbind() is never called, leaving the DRM device registered and memory allocated while the module is unloaded. [Severity: High] This is a pre-existing issue, but if drm_simple_encoder_init() fails, does this leak the bridge in the global bridge_list? drivers/gpu/drm/meson/meson_encoder_dsi.c:meson_encoder_dsi_probe() { ... drm_bridge_add(&meson_encoder_dsi->bridge); meson_encoder_dsi->priv = priv; /* Encoder */ ret = drm_simple_encoder_init(priv->drm, &meson_encoder_dsi->encoder, DRM_MODE_ENCODER_DSI); if (ret) return dev_err_probe(priv->dev, ret, "Failed to init DSI encoder\n"); ... } If drm_simple_encoder_init() fails, the function returns an error without calling drm_bridge_remove(). Because the bridge is a devm allocation, its memory is freed on probe failure, but a dangling pointer remains in the global bridge_list. [Severity: High] This isn't a bug introduced by this patch, but can the teardown order cause a use-after-free when cleaning up components? drivers/gpu/drm/meson/meson_drv.c:meson_drv_bind_master() { ... free_drm: drm_dev_put(drm); meson_encoder_dsi_remove(priv); meson_encoder_hdmi_remove(priv); meson_encoder_cvbs_remove(priv); if (has_components) component_unbind_all(dev, drm); ... } The call to drm_dev_put(drm) drops the final reference and frees the drm device. Then component_unbind_all(dev, drm) passes this newly freed drm pointer to the unbind callbacks, which may attempt to access it. This same sequence also exists in meson_drv_unbind(). -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
