Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Missing error check on devm_drm_panel_bridge_add_typed() leads to an ERR_PTR dereference. - [High] The driver uses the deprecated drm_of_find_panel_or_bridge() API which returns a raw, non-refcounted pointer to a bridge, creating a Use-After-Free risk. -- --- Patch [1]: [PATCH v6] drm/ingenic: fix bridge allocation --- commit 945d263f4d85ed4baf2bf5a4b9f94a4476cfb80e Author: H. Nikolaus Schaller <[email protected]> drm/ingenic: fix bridge allocation This commit fixes the bridge allocation API in ingenic/drm by allocating a stable driver-specific container struct that embeds a struct drm_bridge. It also adjusts the object lifecycle by calling drm_bridge_add before drm_bridge_attach. > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > index 738a80b2550ff..4098192c642bd 100644 > --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c > +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c [ ... ] > @@ -1316,10 +1321,27 @@ 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; > + bridge_alloc = devm_drm_bridge_alloc(priv->dev, > + struct > ingenic_drm_bridge_alloc, > + bridge, [Severity: High] This is a pre-existing issue, but since we are working with the bridge pointer here, should we consider migrating away from the deprecated drm_of_find_panel_or_bridge() API called earlier in this function? Looking at ingenic_drm_bind(), it calls drm_of_find_panel_or_bridge() which returns a non-refcounted bridge. If an external bridge driver unbinds concurrently, the bridge memory could be freed before we finish executing ingenic_drm_bind(), leaving us passing a dangling pointer into drm_bridge_attach(). [Severity: High] This isn't a bug introduced by this patch, but does the bridge pointer need an IS_ERR() check before we use it? A few lines above in ingenic_drm_bind(), devm_drm_panel_bridge_add_typed() is called. If it fails due to memory allocation issues, it returns an ERR_PTR. We blindly assign that to bridge, which eventually gets passed to drm_bridge_attach(). Because drm_bridge_attach() only checks if (!bridge), the ERR_PTR bypasses this check and gets dereferenced when accessing bridge->container, leading to a crash. > + &ingenic_drm_bridge_funcs); > + if (IS_ERR(bridge_alloc)) { > + ret = PTR_ERR(bridge_alloc); > + goto err_drvdata; > + } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/4f30341735d77c77a9a4952abd91879007bc00fd.1787604223.git....@goldelico.com?part=1
