Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Out-of-order delivery of OOB hotplug events due to a Time-of-Check to Time-of-Use (TOCTOU) race condition during connector registration. - [Medium] Permanent memory leak of fwnode references and drm_oob_hotplug_state structs for dynamically generated fwnodes. --
--- Patch [1]: [PATCH] drm/connector: Cache out-of-band hotplug events --- commit 4d3add94a455defa7c034e56e4bb755bf2de93af Author: Sebastian Reichel <[email protected]> drm/connector: Cache out-of-band hotplug events When the USB-C state machine finished negotiating DP AltMode before the DRM device has been probed, the out-of-band hotplug events fired to early and are lost. Without replugging the display or reloading the USB-C driver, the DRM driver assumes nothing is plugged. Link: https://lore.kernel.org/dri-devel/20767137.geO5KgaWL5@diego/ Link: https://lore.kernel.org/dri-devel/[email protected]/ > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 8b4baed060f3a..b69e3776b1539 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c [ ... ] > @@ -829,6 +859,36 @@ void drm_connector_cleanup(struct drm_connector > *connector) > } > EXPORT_SYMBOL(drm_connector_cleanup); > > +/** > + * drm_connector_replay_oob_hotplug_event - send cached OOB HPD event > + * @connector: the connector that should receive the event > + * > + * Send the cached out-of-band hotplug as a new out-of-band hotplug event. > + */ > +static void drm_connector_replay_oob_hotplug_event(struct drm_connector > *connector) > +{ > + struct fwnode_handle *fwnode = connector->fwnode; > + enum drm_connector_status status; > + struct drm_oob_hotplug_state *e; > + bool found = false; > + > + if (!fwnode || !connector->funcs->oob_hotplug_event) > + return; > + > + scoped_guard(mutex, &oob_hotplug_list_lock) { > + list_for_each_entry(e, &oob_hotplug_list, head) { > + if (e->fwnode == fwnode || fwnode->secondary == > e->fwnode) { > + status = e->status; > + found = true; > + break; > + } > + } > + } > + > + if (found) > + connector->funcs->oob_hotplug_event(connector, status); [Severity: High] Does this create a race condition that could result in out-of-order event delivery? When this function drops the oob_hotplug_list_lock after reading the state, a concurrent hardware hotplug event could be processed by drm_connector_oob_hotplug_event() in another thread. If that thread updates the cache and fires the connector callback with a new connected status, would this function then execute its callback with the older, stale cached status, permanently desynchronizing the state? > +} > + [ ... ] > @@ -3671,6 +3737,41 @@ struct drm_connector > *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode) > return found; > } > > +/** > + * drm_connector_record_oob_hotplug_status - Cache OOB hotplug status > + * @fwnode - fwnode for the DRM connector > + * @status - out-of-band status info > + * > + * Cache the latest out-of-band hotplug status for a fwnode so it can be > + * (re)played from when the DRM device is (re)registered after this event > + * arrived. > + */ > +static void drm_connector_record_oob_hotplug_status(struct fwnode_handle > *fwnode, > + enum drm_connector_status > status) > +{ > + struct drm_oob_hotplug_state *e; > + > + if (!fwnode) > + return; > + > + guard(mutex)(&oob_hotplug_list_lock); > + > + list_for_each_entry(e, &oob_hotplug_list, head) { > + if (e->fwnode == fwnode) { > + e->status = status; > + return; > + } > + } > + > + e = kzalloc(sizeof(*e), GFP_KERNEL); > + if (!e) > + return; > + > + e->fwnode = fwnode_handle_get(fwnode); > + e->status = status; > + list_add_tail(&e->head, &oob_hotplug_list); [Severity: Medium] Will this leak memory and fwnode references for dynamically generated fwnodes? It appears that a new state object is allocated and appended to the global oob_hotplug_list, and a reference to the fwnode is taken. However, there does not seem to be any eviction mechanism to remove these items when a connector is destroyed. If dynamic fwnodes are continually added and removed, could this result in unbounded growth of the list and eventual resource exhaustion? > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260821-drm-connector-oob-hotplug-cache-v1-1-e92e146fd...@collabora.com?part=1
