linux-kernel - Re: [PATCH] ASoC: codecs: wcd937x: Fix error handling in wcd937x codec driver

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] [day] [month] [year] [list]

Message-ID: <d09d394f-92d7-420b-b3d4-2eef493d173e@ixit.cz>
Date: Sun, 16 Nov 2025 09:44:33 +0100
From: David Heidelberg <david@...t.cz>
To: Ma Ke <make24@...as.ac.cn>, srini@...nel.org, lgirdwood@...il.com,
 broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
 dmitry.baryshkov@....qualcomm.com
Cc: linux-sound@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
 stable@...r.kernel.org
Subject: Re: [PATCH] ASoC: codecs: wcd937x: Fix error handling in wcd937x
 codec driver

On 16/11/2025 07:16, Ma Ke wrote:
> In wcd937x_bind(), the driver calls of_sdw_find_device_by_node() to
> obtain references to RX and TX SoundWire devices, which increment the
> device reference counts. However, the corresponding put_device() are
> missing in both the error paths and the normal unbind path in
> wcd937x_unbind().
> 
> Add proper error handling with put_device() calls in all error paths
> of wcd937x_bind() and ensure devices are released in wcd937x_unbind().
> 
> Found by code review.
> 
> Cc: stable@...r.kernel.org
> Fixes: 772ed12bd04e ("ASoC: codecs: wcdxxxx: use of_sdw_find_device_by_node helper")
> Signed-off-by: Ma Ke <make24@...as.ac.cn>
> ---
>   sound/soc/codecs/wcd937x.c | 43 ++++++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 9 deletions(-)
> 

[...]

> @@ -2809,10 +2816,26 @@ static int wcd937x_bind(struct device *dev)
>   
>   	ret = snd_soc_register_component(dev, &soc_codec_dev_wcd937x,
>   					 wcd937x_dais, ARRAY_SIZE(wcd937x_dais));
> -	if (ret)
> +	if (ret) {
>   		dev_err(dev, "Codec registration failed\n");
> +		goto err_remove_link3;
> +	}
>   
>   	return ret;
> +
> +err_remove_link3:
> +	device_link_remove(dev, wcd937x->rxdev);
> +err_remove_link2:
> +	device_link_remove(dev, wcd937x->txdev);
> +err_remove_link1:
> +	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
> +err_put_txdev:
> +	put_device(wcd937x->txdev);
> +err_put_rxdev:
> +	put_device(wcd937x->rxdev);
> +err_component_unbind:
> +	component_unbind_all(dev, wcd937x);
> +	return ret;
>   }
>   
>   static void wcd937x_unbind(struct device *dev)
> @@ -2825,6 +2848,8 @@ static void wcd937x_unbind(struct device *dev)
>   	device_link_remove(wcd937x->rxdev, wcd937x->txdev);
>   	component_unbind_all(dev, wcd937x);
>   	mutex_destroy(&wcd937x->micb_lock);
> +	put_device(wcd937x->txdev);
> +	put_device(wcd937x->rxdev);

Shouldn't component_unbind_all be placed here too?

Anyway, current changes (applies with or without the mentioned change):

Reviewed-by: David Heidelberg <david@...t.cz>

Thank you!

>   }
>   
>   static const struct component_master_ops wcd937x_comp_ops = {

-- 
David Heidelberg