Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] Drivers calling of_drm_find_panel() directly leak the returned panel reference. Pre-existing issues: - [High] TOCTOU race in drm_panel_add_follower() when acquiring the panel's underlying device, leading to a potential Use-After-Free of panel->dev. -- --- Patch [4]: [PATCH v2 4/5] drm/panel: find_panel_by_fwnode() return a counted reference --- commit 5cd4c479944cf29f6f5aa4a3e31207bf48dc0567 Author: Albert Esteve <[email protected]> drm/panel: find_panel_by_fwnode() return a counted reference This commit updates find_panel_by_fwnode() to return a refcounted pointer to the panel, mirroring a similar fix made to of_drm_find_panel(). It calls drm_panel_get() under the panel_lock before returning, ensuring the memory is not freed prematurely. Callers are now responsible for balancing this with drm_panel_put(), which is added to drm_panel_remove_follower(). > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index 72cf86253c0cd..a00ae98ed0956 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -542,7 +542,13 @@ int of_drm_get_panel_orientation(const struct > device_node *np, > EXPORT_SYMBOL(of_drm_get_panel_orientation); > #endif > > -/* Find panel by fwnode. This should be identical to of_drm_find_panel(). */ > +/* > + * Find panel by fwnode, returning a counted reference. > + * > + * Behaves identically to of_drm_find_panel(). On success the returned > + * pointer has been passed through drm_panel_get(); the caller must call > + * drm_panel_put() when done with it. > + */ [Severity: Medium] Since the earlier patch in this series updated of_drm_find_panel() to also return a counted reference, do drivers that call it directly need to drop their reference to prevent a memory leak? For example, in bridge drivers like lvds-codec.c: drivers/gpu/drm/bridge/lvds-codec.c:lvds_codec_probe() { panel = of_drm_find_panel(panel_node); of_node_put(panel_node); ... lvds_codec->panel_bridge = devm_drm_panel_bridge_add_typed(dev, panel, lvds_codec->connector_type); } Because devm_drm_panel_bridge_add_typed() takes its own reference to the panel via drm_panel_get(), does this code leak the initial reference returned by of_drm_find_panel() since drm_panel_put() is never called? This same leak pattern appears to exist in other callers as well, such as samsung-dsim.c:samsung_dsim_host_attach() and rcar_du_encoder.c:rcar_du_encoder_init(). > static struct drm_panel *find_panel_by_fwnode(const struct fwnode_handle > *fwnode) > { > struct drm_panel *panel; > @@ -554,6 +560,7 @@ static struct drm_panel *find_panel_by_fwnode(const > struct fwnode_handle *fwnode > > list_for_each_entry(panel, &panel_list, list) { > if (dev_fwnode(panel->dev) == fwnode) { > + drm_panel_get(panel); > mutex_unlock(&panel_lock); [Severity: High] This is a pre-existing issue, but is there a time-of-check to time-of-use race condition when callers attempt to pin the underlying device? When find_panel_by_fwnode() (or find_panel_by_dev()) returns, the panel's internal refcount is incremented under panel_lock, but the underlying struct device (panel->dev) is not pinned. If a concurrent unbind occurs before the caller pins the device, the device memory could be freed. For example, looking at drm_panel_add_follower(): drivers/gpu/drm/drm_panel.c:drm_panel_add_follower() { panel = find_panel_by_dev(follower_dev); if (IS_ERR(panel)) return PTR_ERR(panel); get_device(panel->dev); } If the panel device driver unbinds immediately after panel_lock is dropped, could get_device(panel->dev) trigger a use-after-free on the destroyed device? > return panel; > } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
