Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Use-After-Free of devm-managed resources due to DRM bridge lifetime mismatch - [Medium] Hardware state leak of `ddc_en` GPIO on probe error paths -- commit 9c76898e592dfddd3f50beaa692cc32ae295e2c7 Author: Laurent Pinchart <[email protected]> drm/bridge: display-connector: Fix I2C adapter resource leak If the probe function returns an error after getting the I2C adapter for DDC, the reference to the adapter is never released. Fix it by releasing it in the bridge .destroy() handler. > diff --git a/drivers/gpu/drm/bridge/display-connector.c > b/drivers/gpu/drm/bridge/display-connector.c > index d1e74dd2e0518..eb3412ec30a07 100644 > --- a/drivers/gpu/drm/bridge/display-connector.c > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -43,6 +43,13 @@ static int display_connector_attach(struct drm_bridge > *bridge, > return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL; > } > > +static void display_connector_destroy(struct drm_bridge *bridge) > +{ > + struct display_connector *conn = to_display_connector(bridge); > + > + i2c_put_adapter(conn->bridge.ddc); > +} > + > static enum drm_connector_status display_connector_detect(struct drm_bridge > *bridge) > { > struct display_connector *conn = to_display_connector(bridge); [ ... ] > @@ -439,9 +447,6 @@ static void display_connector_remove(struct > platform_device *pdev) > regulator_disable(conn->supply); > > drm_bridge_remove(&conn->bridge); > - > - if (!IS_ERR(conn->bridge.ddc)) > - i2c_put_adapter(conn->bridge.ddc); > } > > static const struct of_device_id display_connector_match[] = { [Severity: High] This isn't a bug introduced by this patch, but does the bridge lifetime mismatch with devm resources cause a use-after-free? In display_connector_probe(), the bridge is allocated with devm_drm_bridge_alloc(), tying its lifetime to the DRM bridge kref: display_connector_probe() { ... conn = devm_drm_bridge_alloc(&pdev->dev, struct display_connector, bridge, &display_connector_bridge_funcs); ... } However, devm-managed resources like the HPD GPIO and IRQ are tied to the platform device unbind. If the device is unbound via sysfs while the DRM core still holds a reference to the bridge, devres will free the GPIO and IRQ resources. If the DRM core later invokes callbacks like detect or hpd_disable, would it dereference the freed GPIO or attempt to disable the freed IRQ? display_connector_detect() { ... if (gpiod_get_value_cansleep(conn->hpd_gpio)) ... } display_connector_hpd_disable() { ... disable_irq(conn->hpd_irq); ... } [Severity: Medium] This is a pre-existing issue, but is there a hardware state leak on the probe error path? In display_connector_probe(), the DDC enable GPIO is acquired and immediately asserted high: display_connector_probe() { ... if (type == DRM_MODE_CONNECTOR_HDMIA) { ... conn->ddc_en = devm_gpiod_get_optional(&pdev->dev, "ddc-en", GPIOD_OUT_HIGH); if (IS_ERR(conn->ddc_en)) { ... } ret = display_connector_get_supply(pdev, conn, "hdmi-pwr"); if (ret < 0) return dev_err_probe(...); ... } If a subsequent step like display_connector_get_supply() or regulator_enable() fails, the function returns an error. While devres will free the GPIO descriptor, it doesn't revert the physical pin state. Unlike display_connector_remove() which explicitly sets the GPIO to 0, does this error path leave the DDC bus permanently enabled? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
