| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6dbe7287-43ef-4c9e-96fb-78dedc12bbf8@bootlin.com>
Date: Thu, 30 Oct 2025 15:53:38 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Naresh Kamboju <naresh.kamboju@...aro.org>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Hui Pu <Hui.Pu@...ealthcare.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] drm/display: bridge_connector: get/put the stored
bridges
Le 17/10/2025 à 18:15, Luca Ceresoli a écrit :
> drm_bridge_connector_init() takes eight pointers to various bridges, some
> of which can be identical, and stores them in pointers inside struct
> drm_bridge_connector. Get a reference to each of the taken bridges and put
> it on cleanup.
>
> Achieve this by adding a drmm cleanup callback whic puts all the non-NULL
> bridges. Using drmm ensures the cleanup happens on drm_device teardown,
> whichever is the return value of this function.
>
> Four of these pointers (edid, hpd, detect and modes) can be written
> multiple times (up to once per loop iterations), in order to eventually
> store the last matching bridge. So when one of those pointers is
> overwritten, we need to put the reference that we got during the previous
> assignment. Add a drm_bridge_put() before writing them to handle this.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
>
> This patch was added in v2:
> - it is a rewrite of the original (buggy) patch, but simpler after
> realizing thanks to drmm we don't need the temporary pointers as the
> stored pointers will be put just before deallocating
> - compared to the original patch, moved the panel_bridge changes to a
> separate patch for clarity
> ---
> drivers/gpu/drm/display/drm_bridge_connector.c | 52 +++++++++++++++++++-------
> 1 file changed, 39 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index a5bdd6c1064399ece6b19560f145b877c9e0680e..95ccf86527129edaa6fcc75c6202985e73c46da8 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -618,6 +618,20 @@ static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_f
> * Bridge Connector Initialisation
> */
>
> +static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
> +{
> + struct drm_bridge_connector *bridge_connector = (struct drm_bridge_connector *)data;
> +
> + drm_bridge_put(bridge_connector->bridge_edid);
> + drm_bridge_put(bridge_connector->bridge_hpd);
> + drm_bridge_put(bridge_connector->bridge_detect);
> + drm_bridge_put(bridge_connector->bridge_modes);
> + drm_bridge_put(bridge_connector->bridge_hdmi);
> + drm_bridge_put(bridge_connector->bridge_hdmi_audio);
> + drm_bridge_put(bridge_connector->bridge_dp_audio);
> + drm_bridge_put(bridge_connector->bridge_hdmi_cec);
> +}
> +
> /**
> * drm_bridge_connector_init - Initialise a connector for a chain of bridges
> * @drm: the DRM device
> @@ -649,6 +663,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> if (!bridge_connector)
> return ERR_PTR(-ENOMEM);
>
> + ret = drmm_add_action(drm, drm_bridge_connector_put_bridges, bridge_connector);
> + if (ret)
> + return ERR_PTR(ret);
> +
> bridge_connector->encoder = encoder;
>
> /*
> @@ -672,14 +690,22 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> if (!bridge->ycbcr_420_allowed)
> connector->ycbcr_420_allowed = false;
>
> - if (bridge->ops & DRM_BRIDGE_OP_EDID)
> - bridge_connector->bridge_edid = bridge;
> - if (bridge->ops & DRM_BRIDGE_OP_HPD)
> - bridge_connector->bridge_hpd = bridge;
> - if (bridge->ops & DRM_BRIDGE_OP_DETECT)
> - bridge_connector->bridge_detect = bridge;
> - if (bridge->ops & DRM_BRIDGE_OP_MODES)
> - bridge_connector->bridge_modes = bridge;
> + if (bridge->ops & DRM_BRIDGE_OP_EDID) {
> + drm_bridge_put(bridge_connector->bridge_edid);
> + bridge_connector->bridge_edid = drm_bridge_get(bridge);
> + }
> + if (bridge->ops & DRM_BRIDGE_OP_HPD) {
> + drm_bridge_put(bridge_connector->bridge_hpd);
> + bridge_connector->bridge_hpd = drm_bridge_get(bridge);
> + }
> + if (bridge->ops & DRM_BRIDGE_OP_DETECT) {
> + drm_bridge_put(bridge_connector->bridge_detect);
> + bridge_connector->bridge_detect = drm_bridge_get(bridge);
> + }
> + if (bridge->ops & DRM_BRIDGE_OP_MODES) {
> + drm_bridge_put(bridge_connector->bridge_modes);
> + bridge_connector->bridge_modes = drm_bridge_get(bridge);
> + }
> if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
> if (bridge_connector->bridge_hdmi)
> return ERR_PTR(-EBUSY);
> @@ -687,7 +713,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> !bridge->funcs->hdmi_clear_infoframe)
> return ERR_PTR(-EINVAL);
>
> - bridge_connector->bridge_hdmi = bridge;
> + bridge_connector->bridge_hdmi = drm_bridge_get(bridge);
>
> if (bridge->supported_formats)
> supported_formats = bridge->supported_formats;
> @@ -710,7 +736,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> !bridge->funcs->hdmi_audio_shutdown)
> return ERR_PTR(-EINVAL);
>
> - bridge_connector->bridge_hdmi_audio = bridge;
> + bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge);
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
> @@ -728,21 +754,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> !bridge->funcs->dp_audio_shutdown)
> return ERR_PTR(-EINVAL);
>
> - bridge_connector->bridge_dp_audio = bridge;
> + bridge_connector->bridge_dp_audio = drm_bridge_get(bridge);
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
> if (bridge_connector->bridge_hdmi_cec)
> return ERR_PTR(-EBUSY);
>
> - bridge_connector->bridge_hdmi_cec = bridge;
> + bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
> if (bridge_connector->bridge_hdmi_cec)
> return ERR_PTR(-EBUSY);
>
> - bridge_connector->bridge_hdmi_cec = bridge;
> + bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge);
>
> if (!bridge->funcs->hdmi_cec_enable ||
> !bridge->funcs->hdmi_cec_log_addr ||
>
--
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists