| 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: <4b9ac257-1f9e-4e07-bb07-13624cd5ae64@bootlin.com>
Date: Thu, 30 Oct 2025 15:51:48 +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 1/3] Revert "drm/display: bridge_connector: get/put the
stored bridges"
Le 17/10/2025 à 18:15, Luca Ceresoli a écrit :
> This reverts commit 2be300f9a0b6f6b0ae2a90be97e558ec0535be54.
>
> The commit being reverted moved all the bridge_connector->bridge_*
> assignments to just before the final successful return in order to handle
> the bridge refcounting in a clean way.
>
> This introduced a bug, because a bit before the successful return
> drmm_connector_hdmi_cec_register() is called, which calls funcs->init()
> which is drm_bridge_connector_hdmi_cec_init() which needs
> bridge_connector->bridge_hdmi_cec to be set.
>
> The reported bug may be fixed in a relatively simple way, but other similar
> patterns are potentially present, so just revert the offending commit. A
> different approach will be implemented.
>
> Fixes: 2be300f9a0b6 ("drm/display: bridge_connector: get/put the stored bridges")
> Reported-by: Marek Szyprowski <m.szyprowski@...sung.com>
> Closes: https://lore.kernel.org/all/336fbfdd-c424-490e-b5d1-8ee84043dc80@samsung.com/
> Reported-by: Naresh Kamboju <naresh.kamboju@...aro.org>
> Closes: https://lore.kernel.org/r/CA+G9fYuKHp3QgPKjgFY3TfkDdh5Vf=Ae5pCW+eU41Bu=D7th2g@mail.gmail.com
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
Reviewed-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
>
> Changes in v2: none
> ---
> drivers/gpu/drm/display/drm_bridge_connector.c | 114 ++++++++-----------------
> 1 file changed, 36 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 7b18be3ff9a32b362468351835bdab43c3f524f1..a5bdd6c1064399ece6b19560f145b877c9e0680e 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -618,20 +618,6 @@ 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
> @@ -652,15 +638,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> struct drm_bridge_connector *bridge_connector;
> struct drm_connector *connector;
> struct i2c_adapter *ddc = NULL;
> - struct drm_bridge *panel_bridge __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_edid __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_hpd __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_detect __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_modes __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_hdmi __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_hdmi_audio __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_dp_audio __free(drm_bridge_put) = NULL;
> - struct drm_bridge *bridge_hdmi_cec __free(drm_bridge_put) = NULL;
> + struct drm_bridge *panel_bridge = NULL;
> unsigned int supported_formats = BIT(HDMI_COLORSPACE_RGB);
> unsigned int max_bpc = 8;
> bool support_hdcp = false;
> @@ -671,10 +649,6 @@ 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;
>
> /*
> @@ -698,30 +672,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) {
> - drm_bridge_put(bridge_edid);
> - bridge_edid = drm_bridge_get(bridge);
> - }
> - if (bridge->ops & DRM_BRIDGE_OP_HPD) {
> - drm_bridge_put(bridge_hpd);
> - bridge_hpd = drm_bridge_get(bridge);
> - }
> - if (bridge->ops & DRM_BRIDGE_OP_DETECT) {
> - drm_bridge_put(bridge_detect);
> - bridge_detect = drm_bridge_get(bridge);
> - }
> - if (bridge->ops & DRM_BRIDGE_OP_MODES) {
> - drm_bridge_put(bridge_modes);
> - bridge_modes = drm_bridge_get(bridge);
> - }
> + 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_HDMI) {
> - if (bridge_hdmi)
> + if (bridge_connector->bridge_hdmi)
> return ERR_PTR(-EBUSY);
> if (!bridge->funcs->hdmi_write_infoframe ||
> !bridge->funcs->hdmi_clear_infoframe)
> return ERR_PTR(-EINVAL);
>
> - bridge_hdmi = drm_bridge_get(bridge);
> + bridge_connector->bridge_hdmi = bridge;
>
> if (bridge->supported_formats)
> supported_formats = bridge->supported_formats;
> @@ -730,10 +696,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI_AUDIO) {
> - if (bridge_hdmi_audio)
> + if (bridge_connector->bridge_hdmi_audio)
> return ERR_PTR(-EBUSY);
>
> - if (bridge_dp_audio)
> + if (bridge_connector->bridge_dp_audio)
> return ERR_PTR(-EBUSY);
>
> if (!bridge->hdmi_audio_max_i2s_playback_channels &&
> @@ -744,14 +710,14 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> !bridge->funcs->hdmi_audio_shutdown)
> return ERR_PTR(-EINVAL);
>
> - bridge_hdmi_audio = drm_bridge_get(bridge);
> + bridge_connector->bridge_hdmi_audio = bridge;
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
> - if (bridge_dp_audio)
> + if (bridge_connector->bridge_dp_audio)
> return ERR_PTR(-EBUSY);
>
> - if (bridge_hdmi_audio)
> + if (bridge_connector->bridge_hdmi_audio)
> return ERR_PTR(-EBUSY);
>
> if (!bridge->hdmi_audio_max_i2s_playback_channels &&
> @@ -762,7 +728,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> !bridge->funcs->dp_audio_shutdown)
> return ERR_PTR(-EINVAL);
>
> - bridge_dp_audio = drm_bridge_get(bridge);
> + bridge_connector->bridge_dp_audio = bridge;
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
> @@ -773,10 +739,10 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> }
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
> - if (bridge_hdmi_cec)
> + if (bridge_connector->bridge_hdmi_cec)
> return ERR_PTR(-EBUSY);
>
> - bridge_hdmi_cec = drm_bridge_get(bridge);
> + bridge_connector->bridge_hdmi_cec = bridge;
>
> if (!bridge->funcs->hdmi_cec_enable ||
> !bridge->funcs->hdmi_cec_log_addr ||
> @@ -796,7 +762,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> ddc = bridge->ddc;
>
> if (drm_bridge_is_panel(bridge))
> - panel_bridge = drm_bridge_get(bridge);
> + panel_bridge = bridge;
>
> if (bridge->support_hdcp)
> support_hdcp = true;
> @@ -805,13 +771,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> if (connector_type == DRM_MODE_CONNECTOR_Unknown)
> return ERR_PTR(-EINVAL);
>
> - if (bridge_hdmi) {
> + if (bridge_connector->bridge_hdmi) {
> if (!connector->ycbcr_420_allowed)
> supported_formats &= ~BIT(HDMI_COLORSPACE_YUV420);
>
> ret = drmm_connector_hdmi_init(drm, connector,
> - bridge_hdmi->vendor,
> - bridge_hdmi->product,
> + bridge_connector->bridge_hdmi->vendor,
> + bridge_connector->bridge_hdmi->product,
> &drm_bridge_connector_funcs,
> &drm_bridge_connector_hdmi_funcs,
> connector_type, ddc,
> @@ -827,14 +793,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> return ERR_PTR(ret);
> }
>
> - if (bridge_hdmi_audio || bridge_dp_audio) {
> + if (bridge_connector->bridge_hdmi_audio ||
> + bridge_connector->bridge_dp_audio) {
> struct device *dev;
> struct drm_bridge *bridge;
>
> - if (bridge_hdmi_audio)
> - bridge = bridge_hdmi_audio;
> + if (bridge_connector->bridge_hdmi_audio)
> + bridge = bridge_connector->bridge_hdmi_audio;
> else
> - bridge = bridge_dp_audio;
> + bridge = bridge_connector->bridge_dp_audio;
>
> dev = bridge->hdmi_audio_dev;
>
> @@ -848,9 +815,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> return ERR_PTR(ret);
> }
>
> - if (bridge_hdmi_cec &&
> - bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
> - struct drm_bridge *bridge = bridge_hdmi_cec;
> + if (bridge_connector->bridge_hdmi_cec &&
> + bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
> + struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec;
>
> ret = drmm_connector_hdmi_cec_notifier_register(connector,
> NULL,
> @@ -859,9 +826,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> return ERR_PTR(ret);
> }
>
> - if (bridge_hdmi_cec &&
> - bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
> - struct drm_bridge *bridge = bridge_hdmi_cec;
> + if (bridge_connector->bridge_hdmi_cec &&
> + bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
> + struct drm_bridge *bridge = bridge_connector->bridge_hdmi_cec;
>
> ret = drmm_connector_hdmi_cec_register(connector,
> &drm_bridge_connector_hdmi_cec_funcs,
> @@ -874,9 +841,9 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>
> drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
>
> - if (bridge_hpd)
> + if (bridge_connector->bridge_hpd)
> connector->polled = DRM_CONNECTOR_POLL_HPD;
> - else if (bridge_detect)
> + else if (bridge_connector->bridge_detect)
> connector->polled = DRM_CONNECTOR_POLL_CONNECT
> | DRM_CONNECTOR_POLL_DISCONNECT;
>
> @@ -887,15 +854,6 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> IS_ENABLED(CONFIG_DRM_DISPLAY_HDCP_HELPER))
> drm_connector_attach_content_protection_property(connector, true);
>
> - bridge_connector->bridge_edid = drm_bridge_get(bridge_edid);
> - bridge_connector->bridge_hpd = drm_bridge_get(bridge_hpd);
> - bridge_connector->bridge_detect = drm_bridge_get(bridge_detect);
> - bridge_connector->bridge_modes = drm_bridge_get(bridge_modes);
> - bridge_connector->bridge_hdmi = drm_bridge_get(bridge_hdmi);
> - bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge_hdmi_audio);
> - bridge_connector->bridge_dp_audio = drm_bridge_get(bridge_dp_audio);
> - bridge_connector->bridge_hdmi_cec = drm_bridge_get(bridge_hdmi_cec);
> -
> return connector;
> }
> EXPORT_SYMBOL_GPL(drm_bridge_connector_init);
>
--
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists