Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm/display: UAF in CEC unregister due to decoupled lifetimes - [Medium] drm/display: memory leak in drm_bridge_connector_init() error paths --
commit 53e1c33d3e43a19a4088655af78ef18e7e7c5200 Author: Luca Ceresoli <[email protected]> drm/display: bridge-connector: add APIs to add/remove the connector dynamically This commit changes bridge connector allocation to use standard non-managed allocations (kzalloc_obj) instead of drmm_kzalloc(), and introduces dynamic add/remove functions, to prepare for bridge hotplug. > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c > b/drivers/gpu/drm/display/drm_bridge_connector.c > index 1c222e27767d4..2a0065208fb8a 100644 > --- a/drivers/gpu/drm/display/drm_bridge_connector.c > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c > @@ -285,6 +285,7 @@ static void drm_bridge_connector_dynconn_destroy(struct > drm_connector *connector > > drm_connector_cleanup(connector); > drm_bridge_connector_put_bridges(bridge_connector); > + kfree(bridge_connector); > } [Severity: High] Can this cause a use-after-free during device teardown? During initialization, the connector may register drmm managed cleanup actions on the parent drm_device (such as calling drmm_connector_hdmi_cec_notifier_register() inside drm_bridge_connector_initialize()). If a dynamic bridge connector is hot-removed, this function will call kfree() on bridge_connector while those drmm actions are still queued on the drm_device. When the parent drm_device is later torn down, could those managed actions execute and access the already freed connector memory? [ ... ] > @@ -1184,7 +1185,7 @@ struct drm_connector *drm_bridge_connector_init(struct > drm_device *drm, > struct drm_connector *connector; > int ret; > > - bridge_connector = drmm_kzalloc(drm, sizeof(*bridge_connector), > GFP_KERNEL); > + bridge_connector = kzalloc_obj(*bridge_connector); > if (!bridge_connector) > return ERR_PTR(-ENOMEM); > [Severity: Medium] Since bridge_connector is now allocated with kzalloc_obj() instead of drmm_kzalloc(), does this leak bridge_connector if the subsequent call to drm_bridge_connector_initialize() fails and returns an error pointer? [ ... ] > @@ -1199,3 +1200,24 @@ struct drm_connector *drm_bridge_connector_init(struct > drm_device *drm, > return connector; > } > EXPORT_SYMBOL_GPL(drm_bridge_connector_init); > + > +struct drm_connector *drm_bridge_connector_dynamic_add(struct drm_device > *drm, > + struct drm_encoder > *encoder) > +{ > + struct drm_bridge_connector *bridge_connector; > + > + bridge_connector = kzalloc_obj(*bridge_connector); > + if (!bridge_connector) > + return ERR_PTR(-ENOMEM); > + > + return drm_bridge_connector_initialize(bridge_connector, drm, encoder); > +} [Severity: Medium] Similarly here, if drm_bridge_connector_initialize() returns an error, will it leak the bridge_connector allocation instead of freeing it? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
