linux-kernel - Re: [PATCH RESEND] drm/bridge-connector: Fix bridge in drm_connector_hdmi_audio

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aFl9xBBLwSHxCPMq@linaro.org>
Date: Mon, 23 Jun 2025 18:16:04 +0200
From: Stephan Gerhold <stephan.gerhold@...aro.org>
To: Chaoyi Chen <kernel@...kyi.com>,
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: 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>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	Chaoyi Chen <chaoyi.chen@...k-chips.com>
Subject: Re: [PATCH RESEND] drm/bridge-connector: Fix bridge in
 drm_connector_hdmi_audio_init()

On Fri, Jun 20, 2025 at 09:16:16AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
> 
> The bridge used in drm_connector_hdmi_audio_init() does not correctly
> point to the required audio bridge, which lead to incorrect audio
> configuration input.
> 
> Fixes: 231adeda9f67 ("drm/bridge-connector: hook DisplayPort audio support")
> Signed-off-by: Chaoyi Chen <chaoyi.chen@...k-chips.com>

Thanks, this patch fixes the following errors for me on a Qualcomm X1
Elite laptop with v6.16-rc3:

[   10.900256] hdmi-audio-codec hdmi-audio-codec.5.auto: hdmi_codec_probe: Invalid parameters
[   10.901376] hdmi-audio-codec hdmi-audio-codec.4.auto: hdmi_codec_probe: Invalid parameters
[   10.909627] hdmi-audio-codec hdmi-audio-codec.5.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.910813] hdmi-audio-codec hdmi-audio-codec.6.auto: hdmi_codec_probe: Invalid parameters
[   10.918145] hdmi-audio-codec hdmi-audio-codec.4.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.919296] hdmi-audio-codec hdmi-audio-codec.5.auto: hdmi_codec_probe: Invalid parameters
[   10.919298] hdmi-audio-codec hdmi-audio-codec.5.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.928482] hdmi-audio-codec hdmi-audio-codec.6.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.930742] hdmi-audio-codec hdmi-audio-codec.6.auto: hdmi_codec_probe: Invalid parameters
[   10.930744] hdmi-audio-codec hdmi-audio-codec.6.auto: probe with driver hdmi-audio-codec failed with error -22

By de-referencing the "bridge" from the outer scope I think the current
broken code basically de-references some random out-of-bounds memory.
This should be fixed ASAP.

Tested-by: Stephan Gerhold <stephan.gerhold@...aro.org>

> ---
>  drivers/gpu/drm/display/drm_bridge_connector.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 7d2e499ea5de..262e93e07a28 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -708,11 +708,14 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  	if (bridge_connector->bridge_hdmi_audio ||
>  	    bridge_connector->bridge_dp_audio) {
>  		struct device *dev;
> +		struct drm_bridge *bridge;

This is technically redundant since "bridge" is already defined in the
top-level of this function and shadowing it (declaring it again) doesn't
really make it less confusing.

I would suggest either dropping this line and re-assigning the existing
bridge variable (as it was before commit 231adeda9f67
("drm/bridge-connector: hook DisplayPort audio support")), or perhaps
renaming this to

		struct drm_bridge *audio_bridge;

for less confusion.

>  
>  		if (bridge_connector->bridge_hdmi_audio)
> -			dev = bridge_connector->bridge_hdmi_audio->hdmi_audio_dev;
> +			bridge = bridge_connector->bridge_hdmi_audio;
>  		else
> -			dev = bridge_connector->bridge_dp_audio->hdmi_audio_dev;
> +			bridge = bridge_connector->bridge_dp_audio;
> +
> +		dev = bridge->hdmi_audio_dev;

The "dev" variable seems pointless now, just inline
bridge->hdmi_audio_dev in the call below?

>  
>  		ret = drm_connector_hdmi_audio_init(connector, dev,
>  						    &drm_bridge_connector_hdmi_audio_funcs,

Thanks,
Stephan