Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Acquiring a reference in `drm_panel_add()` creates 'zombie' panels on driver probe failure, leading to deferred Use-After-Free of devres-managed resources. Pre-existing issues: - [High] `of_drm_find_panel()` and `find_panel_by_fwnode()` return un-refcounted pointers, leaving consumers vulnerable to TOCTOU Use-After-Free. -- --- Patch [1]: [PATCH v2 1/5] drm/panel: have drm_panel_add/remove manage a list reference --- commit b7da36095272a2101eb01a96e0511080d18be813 Author: Albert Esteve <[email protected]> Subject: drm/panel: have drm_panel_add/remove manage a list reference This commit updates the panel registry to manage a reference count for struct drm_panel objects while they are present in the global list. By calling drm_panel_get() on addition and drm_panel_put() on removal, it attempts to prevent panels from being freed while still linked. > The global registry now holds a counted reference for as long as the > panel is listed, ensuring the object outlives any concurrent lookup. [Severity: High] This is a pre-existing issue, but does the global list reference actually ensure the object outlives the consumer's use of it? Looking at of_drm_find_panel() and find_panel_by_fwnode(), they search the list under panel_lock and return the pointer without acquiring a reference for the caller: of_drm_find_panel() { mutex_lock(&panel_lock); ... mutex_unlock(&panel_lock); return panel; } As soon as panel_lock is unlocked, a concurrent driver unbind could call drm_panel_remove(), which deletes the panel from the list and drops its refcount. If the panel is then freed, won't the consumer thread that called of_drm_find_panel() be left holding a dangling pointer? > 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 acquiring a reference here create zombie panels if a driver's probe fails after adding the panel? For example, if a panel driver successfully calls drm_panel_add() but fails a subsequent probe step (like mipi_dsi_attach) and forgets to call drm_panel_remove(), the device's probe fails. The devres framework will then free all devm-managed hardware resources like regulators, clocks, and GPIOs. However, because the list now holds a reference, the struct drm_panel memory is kept alive and remains in the registry. If a display controller later looks up this zombie panel via of_drm_find_panel() and invokes its prepare() or enable() callbacks, will it dereference those freed devres pointers? > mutex_lock(&panel_lock); > list_add_tail(&panel->list, &panel_list); > mutex_unlock(&panel_lock); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
