Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [High] Modifying drm_panel_add() to take a reference replaces a list-iteration crash with a memory leak and a devres Use-After-Free for drivers that fail to call drm_panel_remove(). Pre-existing issues: - [High] Lookup functions return raw pointers without acquiring references, failing to protect the caller from concurrent panel removal. - [High] Use-After-Free in the drm_panel_follower API when a panel is unbound. -- --- Patch [1]: [PATCH v3 1/4] drm/panel: have drm_panel_add/remove manage a list reference --- commit 2e9c2d76403b7b7e3caee78c96bffbb1f61d8df7 Author: Albert Esteve <[email protected]> drm/panel: have drm_panel_add/remove manage a list reference The commit adds a reference acquisition to drm_panel_add() and a release to drm_panel_remove() to prevent the panel from being freed while still linked in panel_list. The global registry now holds a counted reference for as long as the panel is listed. > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index 2c5649e433dfb..545fe93dc28fe 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -81,6 +81,7 @@ static void drm_panel_init(struct drm_panel *panel, struct > device *dev, > */ > void drm_panel_add(struct drm_panel *panel) > { > + drm_panel_get(panel); [Severity: High] Does taking a reference here replace a list-iteration crash with a memory leak and a devres use-after-free for drivers that fail to call drm_panel_remove()? If a driver calls drm_panel_add() but fails to call drm_panel_remove() on unbind, the new drm_panel_get() reference prevents the struct drm_panel from being freed, leaving it in panel_list. However, the driver's devres teardown still executes, freeing all underlying hardware resources like clocks, regulators, and GPIOs. If another driver later discovers this 'zombie' panel via of_drm_find_panel() and invokes its callbacks, will the panel driver access freed devres resources, causing a use-after-free or hardware lockup? > mutex_lock(&panel_lock); > list_add_tail(&panel->list, &panel_list); > mutex_unlock(&panel_lock); > } This isn't a bug introduced by this patch, but lookup functions return raw pointers without acquiring references, failing to protect the caller from concurrent panel removal: drivers/gpu/drm/drm_panel.c:of_drm_find_panel() { ... list_for_each_entry(panel, &panel_list, list) { if (panel->dev->of_node == np) { mutex_unlock(&panel_lock); return panel; } } ... } [Severity: High] While the commit message claims holding a reference in the registry ensures the object outlives concurrent lookups, does this only protect the list iteration? Once the mutex is unlocked, if a concurrent thread executes drm_panel_remove() (e.g. due to driver unbind), could it drop the registry's reference and potentially free the panel memory while the caller is actively using the unprotected pointer? This is also a pre-existing issue, but there appears to be a use-after-free in the follower API when a panel is unbound: 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); follower->panel = panel; mutex_lock(&panel->follower_lock); ... } [Severity: High] Does acquiring a reference to the panel's underlying device via get_device(panel->dev) fail to keep the devres-allocated struct drm_panel alive? Because the panel is allocated via devm_drm_panel_alloc(), its memory is tied to the driver's devres lifecycle, not the struct device refcount. If the panel driver unbinds, devres frees the struct drm_panel memory regardless of the device refcount, leaving the follower driver with a dangling follower->panel pointer. When the follower unbinds, it calls drm_panel_remove_follower(): drivers/gpu/drm/drm_panel.c:drm_panel_remove_follower() { struct drm_panel *panel = follower->panel; int ret; mutex_lock(&panel->follower_lock); ... } Won't attempting to lock the freed memory (mutex_lock(&panel->follower_lock)) cause a direct use-after-free? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
