[PATCH v3 0/4] ASoC: codecs: wcd937x: add USB Type-C analog audio mux support

[PATCH v3 0/4] ASoC: codecs: wcd937x: add USB Type-C analog audio mux support

[Val Packett]

Currently only wcd939x has support for USB-C SBU audio in mainline, but the
older WCD codecs can do the same thing 939x does with USBSS, with an
external 3rd party mux instead. This series adds support for that based on
wcd939x code.

Tested on sm7325-motorola-dubai.

v3:
  * Zero initialize the device_node with the __free, oops (sashiko-bot)
  * Clear the pointer in wcd93[79]x_mbhc_deinit before tearing down the
    actual wcd_mbhc object (sashiko-bot)
    * Hopefully this is good enough to satisfy the concern about
      getting a typec notification *while removing the codec* which is
      something that has an extremely low chance of ever happening in
      practice anyway. I'm not adding an extra mutex for this. I'm tired.
  * Pull R-b for the commit in the middle but not for the other ones
    because of the changes above.. (prooobably would've been ok but well)
v2: https://lore.kernel.org/all/20261020082631.1509275-1-val@packett.cool/
v1: https://lore.kernel.org/all/20261018231244.1266926-1-val@packett.cool/

Thanks,
~val

---

Val Packett (4):
  ASoC: dt-bindings: qcom,wcd937x: Add USB-C graph properties
  ASoC: codecs: wcd937x: add USB Type-C analog audio mux support
  ASoC: codecs: wcd-mbhc: guard against NULL cfg in
    typec_report_(un)plug
  ASoC: codecs: wcd939x: fix ref leak and NULL deref with USB-C audio

 .../bindings/sound/qcom,wcd937x.yaml          |  15 ++
 sound/soc/codecs/Kconfig                      |   2 +
 sound/soc/codecs/wcd-mbhc-v2.c                |   4 +-
 sound/soc/codecs/wcd937x.c                    | 170 +++++++++++++++++-
 sound/soc/codecs/wcd939x.c                    |  21 ++-
 5 files changed, 198 insertions(+), 14 deletions(-)

-- 
2.55.0

[PATCH v3 1/4] ASoC: dt-bindings: qcom,wcd937x: Add USB-C graph properties

[Val Packett]

This codec can be used with a USB-C SBU mux to support analog headphone
adapters. Describe the mode-switch, orientation-switch and port
properties similarly to wcd939x.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Val Packett <val@packett.cool>
---
 .../devicetree/bindings/sound/qcom,wcd937x.yaml   | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml b/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
index f94203798f24..f15b9a9d90b1 100644
--- a/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,wcd937x.yaml
@@ -25,6 +25,21 @@ properties:
           - const: qcom,wcd9375-codec
           - const: qcom,wcd9370-codec
 
+  mode-switch:
+    description: Flag the port as possible handler of altmode switching
+    type: boolean
+
+  orientation-switch:
+    description: Flag the port as possible handler of orientation switching
+    type: boolean
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      A port node to link the WCD937x Codec node to an external USB-C MUX IC for
+      handling altmode muxing and orientation switching to detect and enable
+      Audio Accessory Mode.
+
   vdd-px-supply:
     description: A reference to the 1.8V I/O supply
 
-- 
2.55.0

[PATCH v3 2/4] ASoC: codecs: wcd937x: add USB Type-C analog audio mux support

[Val Packett]

On mobile devices, this codec is frequently paired with a USB-C SBU mux
like the FSA4480 to support analog headphone adapters. To support this,
register the codec in the Type-C graph whenever one is present in the
device tree, handle the hotplug notifications and report detected GND-MIC
pin swap back to the external mux as Type-C orientation.

Based on code from the wcd939x driver.

Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/Kconfig   |   2 +
 sound/soc/codecs/wcd937x.c | 170 +++++++++++++++++++++++++++++++++++--
 2 files changed, 165 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index ba59dabeeccd..b77e62b0b517 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -2497,11 +2497,13 @@ config SND_SOC_WCD937X
 	depends on SND_SOC_WCD937X_SDW
 	tristate
 	depends on SOUNDWIRE || !SOUNDWIRE
+	depends on TYPEC || !TYPEC
 	select SND_SOC_WCD_CLASSH
 	select SND_SOC_WCD_COMMON
 
 config SND_SOC_WCD937X_SDW
 	tristate "WCD9370/WCD9375 Codec - SDW"
+	depends on TYPEC || !TYPEC
 	select SND_SOC_WCD937X
 	select SND_SOC_WCD_MBHC
 	select REGMAP_IRQ
diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
index 7ffbb445f8ac..9d5d1df41c36 100644
--- a/sound/soc/codecs/wcd937x.c
+++ b/sound/soc/codecs/wcd937x.c
@@ -8,12 +8,15 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of_graph.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
+#include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_altmode.h>
 #include <sound/jack.h>
 #include <sound/pcm_params.h>
 #include <sound/pcm.h>
@@ -82,6 +85,13 @@ struct wcd937x_priv {
 	struct regmap *regmap;
 	/* micb setup lock */
 	struct mutex micb_lock;
+	/* typec handling */
+	bool typec_analog_mux;
+#if IS_ENABLED(CONFIG_TYPEC)
+	enum typec_orientation typec_orientation;
+	unsigned long typec_mode;
+	struct typec_switch *typec_switch;
+#endif /* CONFIG_TYPEC */
 	/* mbhc module */
 	struct wcd_mbhc *wcd_mbhc;
 	struct wcd_mbhc_config mbhc_cfg;
@@ -1869,7 +1879,7 @@ static void wcd937x_mbhc_moisture_config(struct snd_soc_component *component)
 {
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
 
-	if (wcd937x->mbhc_cfg.moist_rref == R_OFF) {
+	if (wcd937x->mbhc_cfg.moist_rref == R_OFF || wcd937x->typec_analog_mux) {
 		snd_soc_component_write_field(component, WCD937X_MBHC_NEW_CTL_2,
 					      WCD937X_M_RTH_CTL_MASK, R_OFF);
 		return;
@@ -1905,7 +1915,7 @@ static bool wcd937x_mbhc_get_moisture_status(struct snd_soc_component *component
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
 	bool ret = false;
 
-	if (wcd937x->mbhc_cfg.moist_rref == R_OFF) {
+	if (wcd937x->mbhc_cfg.moist_rref == R_OFF || wcd937x->typec_analog_mux) {
 		snd_soc_component_write_field(component, WCD937X_MBHC_NEW_CTL_2,
 					      WCD937X_M_RTH_CTL_MASK, R_OFF);
 		goto done;
@@ -2005,6 +2015,7 @@ static int wcd937x_mbhc_init(struct snd_soc_component *component)
 {
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
 	struct wcd_mbhc_intr *intr_ids = &wcd937x->intr_ids;
+	struct wcd_mbhc *mbhc;
 
 	intr_ids->mbhc_sw_intr = regmap_irq_get_virq(wcd937x->irq_chip,
 						     WCD937X_IRQ_MBHC_SW_DET);
@@ -2021,9 +2032,12 @@ static int wcd937x_mbhc_init(struct snd_soc_component *component)
 	intr_ids->hph_right_ocp = regmap_irq_get_virq(wcd937x->irq_chip,
 						      WCD937X_IRQ_HPHR_OCP_INT);
 
-	wcd937x->wcd_mbhc = wcd_mbhc_init(component, &mbhc_cb, intr_ids, wcd_mbhc_fields, true);
-	if (IS_ERR(wcd937x->wcd_mbhc))
-		return PTR_ERR(wcd937x->wcd_mbhc);
+	mbhc = wcd_mbhc_init(component, &mbhc_cb, intr_ids, wcd_mbhc_fields, true);
+	if (IS_ERR(mbhc))
+		return PTR_ERR(mbhc);
+
+	/* May be accessed by platform dev typec callbacks, only set if valid */
+	wcd937x->wcd_mbhc = mbhc;
 
 	snd_soc_add_component_controls(component, impedance_detect_controls,
 				       ARRAY_SIZE(impedance_detect_controls));
@@ -2036,8 +2050,11 @@ static int wcd937x_mbhc_init(struct snd_soc_component *component)
 static void wcd937x_mbhc_deinit(struct snd_soc_component *component)
 {
 	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
+	struct wcd_mbhc *mbhc = wcd937x->wcd_mbhc;
 
-	wcd_mbhc_deinit(wcd937x->wcd_mbhc);
+	/* May be accessed by platform dev typec callbacks, clear it first */
+	wcd937x->wcd_mbhc = NULL;
+	wcd_mbhc_deinit(mbhc);
 }
 
 /* END MBHC */
@@ -2634,6 +2651,40 @@ static const struct snd_soc_component_driver soc_codec_dev_wcd937x = {
 	.endianness = 1,
 };
 
+#if IS_ENABLED(CONFIG_TYPEC)
+/* Get USB-C plug orientation to provide swap event for MBHC */
+static int wcd937x_typec_switch_set(struct typec_switch_dev *sw,
+				    enum typec_orientation orientation)
+{
+	struct wcd937x_priv *wcd937x = typec_switch_get_drvdata(sw);
+
+	wcd937x->typec_orientation = orientation;
+
+	return 0;
+}
+
+static int wcd937x_typec_mux_set(struct typec_mux_dev *mux,
+				 struct typec_mux_state *state)
+{
+	struct wcd937x_priv *wcd937x = typec_mux_get_drvdata(mux);
+	unsigned int previous_mode = wcd937x->typec_mode;
+
+	if (!wcd937x->wcd_mbhc)
+		return -EINVAL;
+
+	if (wcd937x->typec_mode != state->mode) {
+		wcd937x->typec_mode = state->mode;
+
+		if (wcd937x->typec_mode == TYPEC_MODE_AUDIO)
+			return wcd_mbhc_typec_report_plug(wcd937x->wcd_mbhc);
+		else if (previous_mode == TYPEC_MODE_AUDIO)
+			return wcd_mbhc_typec_report_unplug(wcd937x->wcd_mbhc);
+	}
+
+	return 0;
+}
+#endif /* CONFIG_TYPEC */
+
 static bool wcd937x_swap_gnd_mic(struct snd_soc_component *component)
 {
 	int value;
@@ -2644,6 +2695,13 @@ static bool wcd937x_swap_gnd_mic(struct snd_soc_component *component)
 	value = gpiod_get_value(wcd937x->us_euro_gpio);
 	gpiod_set_value(wcd937x->us_euro_gpio, !value);
 
+#if IS_ENABLED(CONFIG_TYPEC)
+	if (wcd937x->typec_analog_mux && wcd937x->typec_switch)
+		typec_switch_set(wcd937x->typec_switch,
+				 wcd937x->typec_orientation == TYPEC_ORIENTATION_REVERSE ?
+					TYPEC_ORIENTATION_NORMAL : TYPEC_ORIENTATION_REVERSE);
+#endif /* CONFIG_TYPEC */
+
 	return true;
 }
 
@@ -2761,10 +2819,24 @@ static int wcd937x_bind(struct device *dev)
 	/* Give the SDW subdevices some more time to settle */
 	usleep_range(5000, 5010);
 
+#if IS_ENABLED(CONFIG_TYPEC)
+	/*
+	 * Get the USB Type-C switch to send GND/MIC swap events.
+	 * typec_switch is fetched now to avoid a probe deadlock since
+	 * the switch depends on the typec_mux register in wcd937x_probe()
+	 */
+	if (wcd937x->typec_analog_mux) {
+		wcd937x->typec_switch = fwnode_typec_switch_get(dev->fwnode);
+		if (IS_ERR(wcd937x->typec_switch))
+			return dev_err_probe(dev, PTR_ERR(wcd937x->typec_switch),
+					     "failed to acquire orientation-switch\n");
+	}
+#endif /* CONFIG_TYPEC */
+
 	ret = component_bind_all(dev, wcd937x);
 	if (ret) {
 		dev_err(dev, "Slave bind failed, ret = %d\n", ret);
-		return ret;
+		goto err_put_typec_switch;
 	}
 
 	wcd937x->rxdev = of_sdw_find_device_by_node(wcd937x->rxnode);
@@ -2852,6 +2924,12 @@ static int wcd937x_bind(struct device *dev)
 	put_device(wcd937x->rxdev);
 err_component_unbind:
 	component_unbind_all(dev, wcd937x);
+err_put_typec_switch:
+#if IS_ENABLED(CONFIG_TYPEC)
+	if (wcd937x->typec_analog_mux)
+		typec_switch_put(wcd937x->typec_switch);
+#endif /* CONFIG_TYPEC */
+
 	return ret;
 }
 
@@ -2867,6 +2945,10 @@ static void wcd937x_unbind(struct device *dev)
 	mutex_destroy(&wcd937x->micb_lock);
 	put_device(wcd937x->txdev);
 	put_device(wcd937x->rxdev);
+#if IS_ENABLED(CONFIG_TYPEC)
+	if (wcd937x->typec_analog_mux)
+		typec_switch_put(wcd937x->typec_switch);
+#endif /* CONFIG_TYPEC */
 }
 
 static const struct component_master_ops wcd937x_comp_ops = {
@@ -2874,6 +2956,61 @@ static const struct component_master_ops wcd937x_comp_ops = {
 	.unbind = wcd937x_unbind,
 };
 
+#if IS_ENABLED(CONFIG_TYPEC)
+static void wcd937x_typec_mux_unregister(void *data)
+{
+	struct typec_mux_dev *typec_mux = data;
+
+	typec_mux_unregister(typec_mux);
+}
+
+static void wcd937x_typec_switch_unregister(void *data)
+{
+	struct typec_switch_dev *typec_sw = data;
+
+	typec_switch_unregister(typec_sw);
+}
+
+static int wcd937x_add_typec(struct wcd937x_priv *wcd937x, struct device *dev)
+{
+	int ret;
+	struct typec_mux_dev *typec_mux;
+	struct typec_switch_dev *typec_sw;
+	struct typec_mux_desc mux_desc = {
+		.drvdata = wcd937x,
+		.fwnode = dev_fwnode(dev),
+		.set = wcd937x_typec_mux_set,
+	};
+	struct typec_switch_desc sw_desc = {
+		.drvdata = wcd937x,
+		.fwnode = dev_fwnode(dev),
+		.set = wcd937x_typec_switch_set,
+	};
+
+	typec_mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(typec_mux))
+		return dev_err_probe(dev, PTR_ERR(typec_mux),
+				     "failed to register typec mux\n");
+
+	ret = devm_add_action_or_reset(dev, wcd937x_typec_mux_unregister,
+				       typec_mux);
+	if (ret)
+		return ret;
+
+	typec_sw = typec_switch_register(dev, &sw_desc);
+	if (IS_ERR(typec_sw))
+		return dev_err_probe(dev, PTR_ERR(typec_sw),
+				     "failed to register typec switch\n");
+
+	ret = devm_add_action_or_reset(dev, wcd937x_typec_switch_unregister,
+				       typec_sw);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+#endif
+
 static int wcd937x_add_slave_components(struct wcd937x_priv *wcd937x,
 					struct device *dev,
 					struct component_match **matchptr)
@@ -2907,6 +3044,9 @@ static int wcd937x_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct wcd937x_priv *wcd937x;
 	struct wcd_mbhc_config *cfg;
+#if IS_ENABLED(CONFIG_TYPEC)
+	struct device_node *np __free(device_node) = NULL;
+#endif /* CONFIG_TYPEC */
 	int ret;
 
 	wcd937x = devm_kzalloc(dev, sizeof(*wcd937x), GFP_KERNEL);
@@ -2951,6 +3091,22 @@ static int wcd937x_probe(struct platform_device *pdev)
 
 	wcd_dt_parse_mbhc_data(dev, &wcd937x->mbhc_cfg);
 
+#if IS_ENABLED(CONFIG_TYPEC)
+	/*
+	 * If the node has a port and a valid remote endpoint,
+	 * consider HP lines as connected to a USB-C SBU mux
+	 */
+	np = of_graph_get_remote_node(dev->of_node, 0, 0);
+	if (np) {
+		wcd937x->typec_analog_mux = true;
+		cfg->typec_analog_mux = true;
+
+		ret = wcd937x_add_typec(wcd937x, dev);
+		if (ret)
+			return ret;
+	}
+#endif /* CONFIG_TYPEC */
+
 	ret = wcd937x_add_slave_components(wcd937x, dev, &match);
 	if (ret)
 		return ret;
-- 
2.55.0

Re: [PATCH v3 2/4] ASoC: codecs: wcd937x: add USB Type-C analog audio mux support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: codecs: wcd937x: race and UAF on wcd_mbhc in wcd937x_typec_mux_set()
- [High] ASoC: codecs: wcd937x: probe deferral deadlock in wcd937x_bind()
--

commit 11dd57a3e1840028ab1f5c083f76fea5c939b494
Author: Val Packett <val@packett.cool>

ASoC: codecs: wcd937x: add USB Type-C analog audio mux support

This commit adds USB Type-C analog audio mux and orientation switch support to
the wcd937x codec driver, registering it in the Type-C graph when present in
device tree and forwarding plug/unplug and GND-MIC swap events between MBHC
and the external mux.

> diff --git a/sound/soc/codecs/wcd937x.c b/sound/soc/codecs/wcd937x.c
> index 7ffbb445f8ac4..9d5d1df41c365 100644
> --- a/sound/soc/codecs/wcd937x.c
> +++ b/sound/soc/codecs/wcd937x.c

[ ... ]

> @@ -2036,8 +2050,11 @@ static int wcd937x_mbhc_init(struct snd_soc_component *component)
>  static void wcd937x_mbhc_deinit(struct snd_soc_component *component)
>  {
>  	struct wcd937x_priv *wcd937x = snd_soc_component_get_drvdata(component);
> +	struct wcd_mbhc *mbhc = wcd937x->wcd_mbhc;
>  
> -	wcd_mbhc_deinit(wcd937x->wcd_mbhc);
> +	/* May be accessed by platform dev typec callbacks, clear it first */
> +	wcd937x->wcd_mbhc = NULL;
> +	wcd_mbhc_deinit(mbhc);
>  }
>  
>  /* END MBHC */
> @@ -2634,6 +2651,40 @@ static const struct snd_soc_component_driver soc_codec_dev_wcd937x = {
>  	.endianness = 1,
>  };
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +/* Get USB-C plug orientation to provide swap event for MBHC */
> +static int wcd937x_typec_switch_set(struct typec_switch_dev *sw,
> +				    enum typec_orientation orientation)
> +{
> +	struct wcd937x_priv *wcd937x = typec_switch_get_drvdata(sw);
> +
> +	wcd937x->typec_orientation = orientation;
> +
> +	return 0;
> +}
> +
> +static int wcd937x_typec_mux_set(struct typec_mux_dev *mux,
> +				 struct typec_mux_state *state)
> +{
> +	struct wcd937x_priv *wcd937x = typec_mux_get_drvdata(mux);
> +	unsigned int previous_mode = wcd937x->typec_mode;
> +
> +	if (!wcd937x->wcd_mbhc)
> +		return -EINVAL;
> +
> +	if (wcd937x->typec_mode != state->mode) {
> +		wcd937x->typec_mode = state->mode;
> +
> +		if (wcd937x->typec_mode == TYPEC_MODE_AUDIO)
> +			return wcd_mbhc_typec_report_plug(wcd937x->wcd_mbhc);
> +		else if (previous_mode == TYPEC_MODE_AUDIO)
> +			return wcd_mbhc_typec_report_unplug(wcd937x->wcd_mbhc);
> +	}
> +
> +	return 0;
> +}
> +#endif /* CONFIG_TYPEC */

[Severity: High]
Can wcd937x_typec_mux_set() race with wcd937x_mbhc_deinit() and cause a
use-after-free on struct wcd_mbhc?

The Type-C mux is registered in wcd937x_probe() for the lifetime of the
platform device, which outlives the ASoC component lifecycle
(wcd937x_soc_codec_probe() and wcd937x_soc_codec_remove()).

If a USB-C mode change invokes typec_mux_set() -> wcd937x_typec_mux_set() on
one CPU while ASoC component removal runs wcd937x_soc_codec_remove() ->
wcd937x_mbhc_deinit() on another CPU, there is no lock synchronizing access
to wcd937x->wcd_mbhc:

CPU0                                     CPU1
wcd937x_typec_mux_set()                  wcd937x_mbhc_deinit()
  if (!wcd937x->wcd_mbhc)
  ...
  wcd_mbhc_typec_report_plug(mbhc)
                                           wcd937x->wcd_mbhc = NULL;
                                           wcd_mbhc_deinit(mbhc)
                                             cancel_work_sync(&mbhc->mbhc_plug_detect_work);
                                             kfree(mbhc);
    mbhc->mbhc_cb->clk_setup(...)
    wcd_mbhc_write_field(...)
    schedule_work(&mbhc->mbhc_plug_detect_work);

This dereferences freed memory and schedules mbhc_plug_detect_work after
cancel_work_sync() and kfree(mbhc) have completed.

Additionally, because wcd937x_typec_mux_set() reads wcd937x->wcd_mbhc twice
without locking, if wcd937x->wcd_mbhc becomes NULL between the initial check
and the call to wcd_mbhc_typec_report_plug(), wcd937x->typec_mode is updated
to state->mode even though wcd_mbhc_typec_report_plug(NULL) returns -EINVAL.

[ ... ]

> @@ -2761,10 +2819,24 @@ static int wcd937x_bind(struct device *dev)
>  	/* Give the SDW subdevices some more time to settle */
>  	usleep_range(5000, 5010);
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +	/*
> +	 * Get the USB Type-C switch to send GND/MIC swap events.
> +	 * typec_switch is fetched now to avoid a probe deadlock since
> +	 * the switch depends on the typec_mux register in wcd937x_probe()
> +	 */
> +	if (wcd937x->typec_analog_mux) {
> +		wcd937x->typec_switch = fwnode_typec_switch_get(dev->fwnode);
> +		if (IS_ERR(wcd937x->typec_switch))
> +			return dev_err_probe(dev, PTR_ERR(wcd937x->typec_switch),
> +					     "failed to acquire orientation-switch\n");
> +	}
> +#endif /* CONFIG_TYPEC */

[Severity: High]
Does fetching typec_switch in wcd937x_bind() still result in a probe deferral
deadlock when the SoundWire slave devices have already probed?

The RX and TX SoundWire slave devices are registered from Device Tree by
sdw_bus_master_add() -> sdw_of_find_slaves(), and wcd9370_probe() in
sound/soc/codecs/wcd937x-sdw.c calls component_add() without depending on
wcd937x_probe(). When built as modules, snd-soc-wcd937x.ko also depends on
symbols exported by snd-soc-wcd937x-sdw.ko, so wcd9370_probe() runs before
wcd937x_probe() (and when built-in, wcd9370_probe() can still finish first if
wcd937x_probe() defers once on regulators or GPIOs).

When both slave components are already added, the call chain in
wcd937x_probe() runs wcd937x_bind() synchronously:

wcd937x_probe()
  wcd937x_add_typec()
    typec_mux_register()
    typec_switch_register()
  component_master_add_with_match()
    try_to_bring_up_aggregate_device()
      wcd937x_bind()
        fwnode_typec_switch_get() -> returns -EPROBE_DEFER

Because wcd937x_probe() has not returned yet, really_probe() has not called
driver_bound() -> driver_deferred_probe_trigger(), so the external Type-C
switch driver waiting on wcd937x's typec_mux has not re-probed. The
-EPROBE_DEFER from fwnode_typec_switch_get() propagates out of
component_master_add_with_match() and fails wcd937x_probe(), triggering devm
cleanup to unregister wcd937x's typec_mux and typec_switch and recreating the
mutual probe deferral deadlock.

[ ... ]

> @@ -2951,6 +3091,22 @@ static int wcd937x_probe(struct platform_device *pdev)
>  
>  	wcd_dt_parse_mbhc_data(dev, &wcd937x->mbhc_cfg);
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +	/*
> +	 * If the node has a port and a valid remote endpoint,
> +	 * consider HP lines as connected to a USB-C SBU mux
> +	 */
> +	np = of_graph_get_remote_node(dev->of_node, 0, 0);
> +	if (np) {
> +		wcd937x->typec_analog_mux = true;
> +		cfg->typec_analog_mux = true;
> +
> +		ret = wcd937x_add_typec(wcd937x, dev);
> +		if (ret)
> +			return ret;
> +	}
> +#endif /* CONFIG_TYPEC */
> +
>  	ret = wcd937x_add_slave_components(wcd937x, dev, &match);
>  	if (ret)
>  		return ret;

[ ... ]

>  	ret = component_master_add_with_match(dev, &wcd937x_comp_ops, match);
>  	if (ret)
>  		return ret;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020170341.1641325-1-val@packett.cool?part=2

[PATCH v3 3/4] ASoC: codecs: wcd-mbhc: guard against NULL cfg in typec_report_(un)plug

[Val Packett]

mbhc->cfg is only set by wcd_mbhc_start() which is called by the codec's
set_jack op. wcd_mbhc_typec_report_plug/unplug are called by the platform
device's typec callbacks, which are not synchronized with codec ops, so
a hotplug could be reported before set_jack was called. Though unlikely
to happen in practice, add a check to be safe.

Found by sashiko-bot via review of wcd937x Type-C integration patches.

Cc: stable@vger.kernel.org
Fixes: 0c105997eefd ("ASoC: codec: wcd-mbhc-v2: add support when connected behind an USB-C audio mux")
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/wcd-mbhc-v2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
index 850be84d65ce..fd40c1686d66 100644
--- a/sound/soc/codecs/wcd-mbhc-v2.c
+++ b/sound/soc/codecs/wcd-mbhc-v2.c
@@ -594,7 +594,7 @@ static irqreturn_t wcd_mbhc_mech_plug_detect_irq(int irq, void *data)
 int wcd_mbhc_typec_report_unplug(struct wcd_mbhc *mbhc)
 {
 
-	if (!mbhc || !mbhc->cfg->typec_analog_mux)
+	if (!mbhc || !mbhc->cfg || !mbhc->cfg->typec_analog_mux)
 		return -EINVAL;
 
 	if (mbhc->mbhc_cb->clk_setup)
@@ -611,7 +611,7 @@ EXPORT_SYMBOL_GPL(wcd_mbhc_typec_report_unplug);
 
 int wcd_mbhc_typec_report_plug(struct wcd_mbhc *mbhc)
 {
-	if (!mbhc || !mbhc->cfg->typec_analog_mux)
+	if (!mbhc || !mbhc->cfg || !mbhc->cfg->typec_analog_mux)
 		return -EINVAL;
 
 	if (mbhc->mbhc_cb->clk_setup)
-- 
2.55.0

Re: [PATCH v3 3/4] ASoC: codecs: wcd-mbhc: guard against NULL cfg in typec_report_(un)plug

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: codecs: wcd-mbhc: UAF race between typec_report_(un)plug() and deinit()
- [Medium] ASoC: wcd-mbhc: race between wcd_mbhc_start() and typec_report_(un)plug()
--

commit 6517f3fdf1f47c12dda04120bfa0403f19390817
Author: Val Packett <val@packett.cool>

ASoC: codecs: wcd-mbhc: guard against NULL cfg in typec_report_(un)plug

This commit adds a NULL check for mbhc->cfg in
wcd_mbhc_typec_report_unplug() and wcd_mbhc_typec_report_plug() to guard
against Type-C callbacks firing before wcd_mbhc_start() is called.

> diff --git a/sound/soc/codecs/wcd-mbhc-v2.c b/sound/soc/codecs/wcd-mbhc-v2.c
> index 850be84d65ceb..fd40c1686d66a 100644
> --- a/sound/soc/codecs/wcd-mbhc-v2.c
> +++ b/sound/soc/codecs/wcd-mbhc-v2.c
> @@ -594,7 +594,7 @@ static irqreturn_t wcd_mbhc_mech_plug_detect_irq(int irq, void *data)
>  int wcd_mbhc_typec_report_unplug(struct wcd_mbhc *mbhc)
>  {
>  
> -	if (!mbhc || !mbhc->cfg->typec_analog_mux)
> +	if (!mbhc || !mbhc->cfg || !mbhc->cfg->typec_analog_mux)
>  		return -EINVAL;
>  
>  	if (mbhc->mbhc_cb->clk_setup)
> @@ -611,7 +611,7 @@ EXPORT_SYMBOL_GPL(wcd_mbhc_typec_report_unplug);
>  
>  int wcd_mbhc_typec_report_plug(struct wcd_mbhc *mbhc)
>  {
> -	if (!mbhc || !mbhc->cfg->typec_analog_mux)
> +	if (!mbhc || !mbhc->cfg || !mbhc->cfg->typec_analog_mux)
>  		return -EINVAL;
>  
>  	if (mbhc->mbhc_cb->clk_setup)

[Severity: Medium]
Is checking !mbhc->cfg without holding mbhc->lock sufficient to synchronize
wcd_mbhc_typec_report_plug() and wcd_mbhc_typec_report_unplug() against
wcd_mbhc_start(), wcd_mbhc_initialise(), and wcd_mbhc_stop()?

In sound/soc/codecs/wcd-mbhc-v2.c, wcd_mbhc_start() assigns mbhc->cfg
without holding mbhc->lock, before setting mbhc->jack and before
wcd_mbhc_initialise() initializes the hardware:

sound/soc/codecs/wcd-mbhc-v2.c:wcd_mbhc_start() {
	if (!mbhc || !cfg || !jack)
		return -EINVAL;

	mbhc->cfg = cfg;
	mbhc->jack = jack;

	return wcd_mbhc_initialise(mbhc);
}

wcd_mbhc_initialise() then calls pm_runtime_get_sync() before taking
mbhc->lock and configuring the registers:

sound/soc/codecs/wcd-mbhc-v2.c:wcd_mbhc_initialise() {
	ret = pm_runtime_get_sync(component->dev);
	if (ret < 0 && ret != -EACCES) {
		...
		return ret;
	}

	mutex_lock(&mbhc->lock);
	...
	/* Plug detect is triggered manually if analog goes through USBCC */
	if (mbhc->cfg->typec_analog_mux)
		wcd_mbhc_write_field(mbhc, WCD_MBHC_L_DET_EN, 0);
	else
		wcd_mbhc_write_field(mbhc, WCD_MBHC_L_DET_EN, 1);
	...
	if (mbhc->mbhc_cb->clk_setup)
		mbhc->mbhc_cb->clk_setup(component, false);
	mutex_unlock(&mbhc->lock);
}

If wcd937x_typec_mux_set() or wcd939x_typec_mux_set() calls
wcd_mbhc_typec_report_plug() while wcd_mbhc_start() is in progress,
wcd_mbhc_typec_report_plug() can observe mbhc->cfg != NULL, enable the MBHC
clock, set WCD_MBHC_L_DET_EN to 1, and schedule mbhc_plug_detect_work
before wcd_mbhc_initialise() has configured the MBHC registers or
mbhc->swap_thr. When wcd_mbhc_initialise() then runs under mbhc->lock, it
will clear WCD_MBHC_L_DET_EN back to 0 and disable clk_setup, breaking the
plug detection state.

Additionally, mbhc->cfg is not cleared if wcd_mbhc_initialise() fails or
when wcd_mbhc_stop() runs. Could wcd_mbhc_typec_report_plug() and
wcd_mbhc_typec_report_unplug() still execute when MBHC is uninitialized or
stopped?

[Severity: High]
Can wcd_mbhc_typec_report_plug() and wcd_mbhc_typec_report_unplug() also
race with wcd_mbhc_deinit(), causing a use-after-free on struct wcd_mbhc
and mbhc->mbhc_plug_detect_work?

As the commit message notes, the platform device's Type-C callbacks are not
synchronized with codec ops. In sound/soc/codecs/wcd937x.c (and similarly
in wcd939x.c), wcd937x_typec_mux_set() checks wcd937x->wcd_mbhc and passes
the pointer to wcd_mbhc_typec_report_plug() or
wcd_mbhc_typec_report_unplug() without holding a lock that serializes
against teardown:

sound/soc/codecs/wcd937x.c:wcd937x_typec_mux_set() {
	if (!wcd937x->wcd_mbhc)
		return -EINVAL;

	if (wcd937x->typec_mode != state->mode) {
		wcd937x->typec_mode = state->mode;

		if (wcd937x->typec_mode == TYPEC_MODE_AUDIO)
			return wcd_mbhc_typec_report_plug(wcd937x->wcd_mbhc);
		else if (previous_mode == TYPEC_MODE_AUDIO)
			return wcd_mbhc_typec_report_unplug(wcd937x->wcd_mbhc);
	}
}

During ASoC component removal, wcd937x_soc_codec_remove() calls
wcd937x_mbhc_deinit() -> wcd_mbhc_deinit():

sound/soc/codecs/wcd937x.c:wcd937x_mbhc_deinit() {
	/* May be accessed by platform dev typec callbacks, clear it first */
	wcd937x->wcd_mbhc = NULL;
	wcd_mbhc_deinit(mbhc);
}

sound/soc/codecs/wcd-mbhc-v2.c:wcd_mbhc_deinit() {
	...
	mutex_lock(&mbhc->lock);
	wcd_cancel_hs_detect_plug(mbhc,	&mbhc->correct_plug_swch);
	cancel_work_sync(&mbhc->mbhc_plug_detect_work);
	mutex_unlock(&mbhc->lock);

	kfree(mbhc);
}

If wcd937x_typec_mux_set() has already loaded wcd937x->wcd_mbhc before
wcd937x_mbhc_deinit() clears it, wcd_mbhc_deinit() can finish
cancel_work_sync(&mbhc->mbhc_plug_detect_work) and kfree(mbhc) while
wcd_mbhc_typec_report_plug() or wcd_mbhc_typec_report_unplug() is still
running, dereferencing freed mbhc fields and calling
schedule_work(&mbhc->mbhc_plug_detect_work) on freed memory.

Should the Type-C mux callback and MBHC teardown be synchronized with a
lock that spans the wcd_mbhc_typec_report_plug() /
wcd_mbhc_typec_report_unplug() call and wcd_mbhc_deinit()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020170341.1641325-1-val@packett.cool?part=3

[PATCH v3 4/4] ASoC: codecs: wcd939x: fix ref leak and NULL deref with USB-C audio

[Val Packett]

Some code issues were found during review of a port of typec code from
wcd939x to wcd937x, pointed out by sashiko-bot:

* wcd939x->wcd_mbhc may be accessed by the typec callbacks which are
  independent of the ASoC codec lifecycle, don't leave it as NULL/PTR_ERR;
* wcd939x->typec_switch reference was leaked on unbind, put it there;
* the device_node reference returned by of_graph_get_remote_node() was
  leaked, add a __free annotation to the declaration.

Fix them in wcd939x as well.

Cc: stable@vger.kernel.org
Fixes: 10f514bd172a ("ASoC: codecs: Add WCD939x Codec driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/wcd939x.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/wcd939x.c b/sound/soc/codecs/wcd939x.c
index 2d463f062e6d..b0b09468f98a 100644
--- a/sound/soc/codecs/wcd939x.c
+++ b/sound/soc/codecs/wcd939x.c
@@ -2470,6 +2470,7 @@ static int wcd939x_mbhc_init(struct snd_soc_component *component)
 {
 	struct wcd939x_priv *wcd939x = snd_soc_component_get_drvdata(component);
 	struct wcd_mbhc_intr *intr_ids = &wcd939x->intr_ids;
+	struct wcd_mbhc *mbhc;
 
 	intr_ids->mbhc_sw_intr = regmap_irq_get_virq(wcd939x->irq_chip,
 						     WCD939X_IRQ_MBHC_SW_DET);
@@ -2486,9 +2487,12 @@ static int wcd939x_mbhc_init(struct snd_soc_component *component)
 	intr_ids->hph_right_ocp = regmap_irq_get_virq(wcd939x->irq_chip,
 						      WCD939X_IRQ_HPHR_OCP_INT);
 
-	wcd939x->wcd_mbhc = wcd_mbhc_init(component, &mbhc_cb, intr_ids, wcd_mbhc_fields, true);
-	if (IS_ERR(wcd939x->wcd_mbhc))
-		return PTR_ERR(wcd939x->wcd_mbhc);
+	mbhc = wcd_mbhc_init(component, &mbhc_cb, intr_ids, wcd_mbhc_fields, true);
+	if (IS_ERR(mbhc))
+		return PTR_ERR(mbhc);
+
+	/* May be accessed by platform dev typec callbacks, only set if valid */
+	wcd939x->wcd_mbhc = mbhc;
 
 	snd_soc_add_component_controls(component, impedance_detect_controls,
 				       ARRAY_SIZE(impedance_detect_controls));
@@ -2501,8 +2505,11 @@ static int wcd939x_mbhc_init(struct snd_soc_component *component)
 static void wcd939x_mbhc_deinit(struct snd_soc_component *component)
 {
 	struct wcd939x_priv *wcd939x = snd_soc_component_get_drvdata(component);
+	struct wcd_mbhc *mbhc = wcd939x->wcd_mbhc;
 
-	wcd_mbhc_deinit(wcd939x->wcd_mbhc);
+	/* May be accessed by platform dev typec callbacks, clear it first */
+	wcd939x->wcd_mbhc = NULL;
+	wcd_mbhc_deinit(mbhc);
 }
 
 /* END MBHC */
@@ -3175,7 +3182,7 @@ static int wcd939x_populate_dt_data(struct wcd939x_priv *wcd939x, struct device
 {
 	struct wcd_mbhc_config *cfg = &wcd939x->mbhc_cfg;
 #if IS_ENABLED(CONFIG_TYPEC)
-	struct device_node *np;
+	struct device_node *np __free(device_node) = NULL;
 #endif /* CONFIG_TYPEC */
 	int ret;
 
@@ -3438,6 +3445,10 @@ static void wcd939x_unbind(struct device *dev)
 	put_device(wcd939x->txdev);
 	put_device(wcd939x->rxdev);
 	component_unbind_all(dev, wcd939x);
+#if IS_ENABLED(CONFIG_TYPEC)
+	if (wcd939x->typec_analog_mux)
+		typec_switch_put(wcd939x->typec_switch);
+#endif /* CONFIG_TYPEC */
 }
 
 static const struct component_master_ops wcd939x_comp_ops = {
-- 
2.55.0