From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C44BA370D7B for ; Sun, 5 Jul 2026 23:38:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783294731; cv=none; b=YgcaprFD1Za7Rs+Lmdt9N0kBW8UJGWSH06+bMrEijlobrsK09vRjEHMz39B2jbA+EJLsFnRHGJ9ykwVZHF2TTIel4RAh7W18lCBnlhgyWOxlQPryPa2OVjVigTJdUrjlZNfS1cEJdqz5etqeA7/u1V5vt/xtVzogNcUsTyrAsK4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783294731; c=relaxed/simple; bh=94ZGNpcgMFcHz7V/t7ZLK6w6YKEaAwrUVIrOqlV0goE=; h=Message-ID:From:To:Cc:Subject:Date:In-Reply-To:References: Content-Type:MIME-Version; b=urq/D3aEa1Kj3aLdK7W2tgCyMwYxP2tJ8crcmdYH0OgmhL3te0ZKcA6nXNzsKbCh3FC1zrSxtMZ+xdH1kcrfTKGiCrjkHbEZjDdjTSBToDt/LTIgygkh/vCPIWBrnZrMSxRGHu0+jPrzqtuRc+P3Y+MjpX8/VM6yis5sU5xuBt0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=uCjoqEZh; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="uCjoqEZh" Received: from killaraus.ideasonboard.com (2001-14ba-70f3-e800--a06.rev.dnainternet.fi [IPv6:2001:14ba:70f3:e800::a06]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 921FEBE; Mon, 6 Jul 2026 01:37:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1783294678; bh=94ZGNpcgMFcHz7V/t7ZLK6w6YKEaAwrUVIrOqlV0goE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uCjoqEZhusctEsIdiLEgNu7Tjcwo93hV7ipdLqS76J2mo+/ZzONDbKtA6j2U+2lEv 3fPjM79J5kcEOnpCU8jeJv1Ej6QIwO55DQZtmjQsrbXF9pViGu5hrFD24NQiHnKtrZ J5NpwH4mrnfKhCxDM7ja8qglLsfqbhHJpfBBiGxw= Message-ID: <7f42593b8a35e68960f2f6570ea3d948@killaraus.ideasonboard.com> From: Laurent Pinchart To: sashiko-reviews@lists.linux.dev Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, wsa+renesas@sang-engineering.com, robh@kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 2/4] drm/bridge: display-connector: Fix I2C adapter resource leak Date: Mon, 06 Jul 2026 02:38:43 +0300 In-Reply-To: <20260705214826.628831F000E9@smtp.kernel.org> References: <20260705213542.28987-1-laurent.pinchart+renesas@ideasonboard.com> <20260705213542.28987-3-laurent.pinchart+renesas@ideasonboard.com> <20260705214826.628831F000E9@smtp.kernel.org> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: >=20 > Pre-existing issues: > - [High] Use-After-Free of devm-managed resources due to DRM bridge lifetim= e mismatch > - [Medium] Hardware state leak of `ddc_en` GPIO on probe error paths > -- >=20 > commit 9c76898e592dfddd3f50beaa692cc32ae295e2c7 > Author: Laurent Pinchart >=20 > drm/bridge: display-connector: Fix I2C adapter resource leak >=20 > 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. >=20 > > 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; > > } > > =20 > > +static void display_connector_destroy(struct drm_bridge *bridge) > > +{ > > + struct display_connector *conn =3D to_display_connector(bridge); > > + > > + i2c_put_adapter(conn->bridge.ddc); > > +} > > + > > static enum drm_connector_status display_connector_detect(struct drm_bri= dge *bridge) > > { > > struct display_connector *conn =3D to_display_connector(bridge); >=20 > [ ... ] >=20 > > @@ -439,9 +447,6 @@ static void display_connector_remove(struct platform_= device *pdev) > > regulator_disable(conn->supply); > > =20 > > drm_bridge_remove(&conn->bridge); > > - > > - if (!IS_ERR(conn->bridge.ddc)) > > - i2c_put_adapter(conn->bridge.ddc); > > } > > =20 > > static const struct of_device_id display_connector_match[] =3D { >=20 > [Severity: High] > This isn't a bug introduced by this patch, but does the bridge lifetime=20 > mismatch with devm resources cause a use-after-free? >=20 > In display_connector_probe(), the bridge is allocated with=20 > devm_drm_bridge_alloc(), tying its lifetime to the DRM bridge kref: >=20 > display_connector_probe() { > ... > conn =3D devm_drm_bridge_alloc(&pdev->dev, struct display_connector, br= idge, > &display_connector_bridge_funcs); > ... > } >=20 > However, devm-managed resources like the HPD GPIO and IRQ are tied to the=20 > platform device unbind. If the device is unbound via sysfs while the DRM=20 > core still holds a reference to the bridge, devres will free the GPIO and=20 > IRQ resources. >=20 > If the DRM core later invokes callbacks like detect or hpd_disable, would i= t=20 > dereference the freed GPIO or attempt to disable the freed IRQ? >=20 > display_connector_detect() { > ... > if (gpiod_get_value_cansleep(conn->hpd_gpio)) > ... > } >=20 > display_connector_hpd_disable() { > ... > disable_irq(conn->hpd_irq); > ... > } >=20 > [Severity: Medium] > This is a pre-existing issue, but is there a hardware state leak on the=20 > probe error path? >=20 > In display_connector_probe(), the DDC enable GPIO is acquired and=20 > immediately asserted high: >=20 > display_connector_probe() { > ... > if (type =3D=3D DRM_MODE_CONNECTOR_HDMIA) { > ... > conn->ddc_en =3D devm_gpiod_get_optional(&pdev->dev, "ddc-en", > GPIOD_OUT_HIGH); >=20 > if (IS_ERR(conn->ddc_en)) { > ... > } >=20 > ret =3D display_connector_get_supply(pdev, conn, "hdmi-pwr"); > if (ret < 0) > return dev_err_probe(...); > ... > } >=20 > If a subsequent step like display_connector_get_supply() or=20 > regulator_enable() fails, the function returns an error. While devres will = > free the GPIO descriptor, it doesn't revert the physical pin state. >=20 > Unlike display_connector_remove() which explicitly sets the GPIO to 0, does= =20 > this error path leave the DDC bus permanently enabled? >=20