Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] This problem wasn't introduced by this patch, but there is a missing IS_ERR() check on the return value of devm_drm_panel_bridge_add_typed() which leads to an error pointer dereference. -- --- Patch [1]: [PATCH v3] drm/ingenic: fix bridge allocation --- commit 4187db84355de6d4a1d09fa81233bcc5ab620c4b Author: H. Nikolaus Schaller <[email protected]> Subject: drm/ingenic: fix bridge allocation This patch updates the ingenic/drm driver to use the new devm_drm_bridge_alloc() API for bridge allocation. It fixes an issue where the bridge was corrupted or not allocated properly under the new API, causing use-after-free warnings. > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > index 738a80b2550ff..d1321a9c0f0b0 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c [ ... ] > @@ -1316,10 +1316,25 @@ static int ingenic_drm_bind(struct device *dev, bool > has_components) > > drm_encoder_helper_add(encoder, > &ingenic_drm_encoder_helper_funcs); > > - ib->bridge.funcs = &ingenic_drm_bridge_funcs; > + ib->bridge = devm_drm_bridge_alloc(priv->dev, struct drm_bridge, > + base, > &ingenic_drm_bridge_funcs); > + if (IS_ERR(ib->bridge)) { > + ret = PTR_ERR(ib->bridge); > + goto err_drvdata; > + } > + > + ib->bridge->of_node = priv->dev->of_node; > ib->next_bridge = bridge; [Severity: High] This isn't a bug introduced by this patch, but if devm_drm_panel_bridge_add_typed() fails earlier in ingenic_drm_bind(), it returns an ERR_PTR. Does this assignment store that unvalidated error pointer into ib->next_bridge without an IS_ERR() check? When the bridge attach callback executes: static int ingenic_drm_bridge_attach(...) { ... return drm_bridge_attach(encoder, ib->next_bridge, bridge, flags); } Would this pass the ERR_PTR as the bridge argument to drm_bridge_attach(), which bypasses the NULL check and dereferences it? int drm_bridge_attach(..., struct drm_bridge *bridge, ...) { if (!encoder || !bridge) return -EINVAL; if (!bridge->container) ... } Could this lead to a kernel panic from an invalid memory access? > - ret = drm_bridge_attach(encoder, &ib->bridge, NULL, > + ib->bridge->interlace_allowed = true; > + > + ret = devm_drm_bridge_add(priv->dev, ib->bridge); > + if (ret) { > + dev_err(dev, "Failed to register DRM bridge: %d\n", > ret); > + goto err_drvdata; > + } > + > + ret = drm_bridge_attach(encoder, ib->bridge, NULL, > DRM_BRIDGE_ATTACH_NO_CONNECTOR); > if (ret) { > dev_err(dev, "Unable to attach bridge\n"); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/a3b64245250cd092776cf23cddffe18318b48995.1787494377.git....@goldelico.com?part=1
