Hello, This is the friendly mail bot of Laurent Pinchart. You have sent him what appears to be an e-mail generated by an LLM. In order to preserve his mental health, the e-mail has been automatically ignored.
Thank you, Laurent's spam bot On Sun, 05 Jul 2026 21:48:25 +0000, [email protected] wrote: > 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? >
