Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Removing `drm_dp_aux_unregister()` from the I2C `remove` function causes a use-after-free and leaves a zombie DP AUX adapter if the I2C driver is unbound while the DRM device remains attached. --
--- Patch [3]: [PATCH v6 03/13] drm/bridge: it6505: unregister DP AUX adapter on bridge detach --- commit d6c69e9e1c11461da60c039acb301f1120930d47 Author: Daniel Golle <[email protected]> drm/bridge: it6505: unregister DP AUX adapter on bridge detach This commit moves the DP AUX adapter unregistration from the I2C remove routine to the DRM bridge detach and attach error paths, to avoid registering the adapter multiple times during unbind/rebind cycles. > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > index 7c61b02cc7e60..c10cc6a786b5d 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c [ ... ] > @@ -3106,6 +3107,7 @@ static void it6505_bridge_detach(struct drm_bridge > *bridge) > > flush_work(&it6505->link_works); > it6505_remove_notifier_module(it6505); > + drm_dp_aux_unregister(&it6505->aux); > } > [ ... ] > @@ -3654,7 +3656,6 @@ static void it6505_i2c_remove(struct i2c_client *client) > struct it6505 *it6505 = i2c_get_clientdata(client); > > drm_bridge_remove(&it6505->bridge); > - drm_dp_aux_unregister(&it6505->aux); [Severity: High] Does removing drm_dp_aux_unregister() here cause a use-after-free if the I2C driver is unbound while the DRM device is still attached? If the I2C driver is unbound (e.g., via sysfs) while the DRM display controller is still bound, it6505_i2c_remove() is called. The drm_bridge_remove() call only moves the bridge to a lingering list and does not immediately call it6505_bridge_detach(). Because drm_dp_aux_unregister() was removed from this path, the AUX adapter remains registered. When the remove function returns, devres frees the it6505 structure. Could a local user trigger a use-after-free by accessing the leaked /dev/drm_dp_auxN character device or I2C sysfs nodes which now point to the freed structure? Since i2c_del_adapter() and drm_dp_aux_unregister_devnode() both contain safety guards against unregistered adapters, is it safer to leave drm_dp_aux_unregister() in the remove path as well to ensure it is always unregistered before devres frees the memory? > it6505_debugfs_remove(it6505); > it6505_remove_notifier_module(it6505); > disable_irq(it6505->irq); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
