ASoC: rt5663: Cancel jack detect work on unbind

ASoC: rt5663: Cancel jack detect work on unbind

jack_detect_work and jd_unplug_work live in struct rt5663_priv, which devres frees after rt5663_i2c_remove() returns. The IRQ queues the first work, a button press queues the second, and jd_unplug_work rearms itself while the jack stays inserted. Both callbacks load rt5663->component before checking it, and nothing clears that pointer. rt5663_suspend() cancels the works; unbind does not.

rt5663_remove() alone does not cover unbind. snd_soc_del_component_unlocked() skips it when component->card is NULL, which is true if the codec was never bound or the card was already unbound. The jack IRQ is requested before the component is registered, so it can arm the work with no card, and it stays live across a card unbind.

rt5663_i2c_remove() alone is not enough either. rt5663_set_jack_detect() queues jack_detect_work even when clearing the jack. On a bound I2C unbind that call comes from snd_soc_link_exit() after rt5663_i2c_remove() has returned.

Cancel both works after free_irq() in rt5663_i2c_remove(), and again in rt5663_remove(). Cancel jack_detect_work first; it can queue jd_unplug_work.

This issue was identified during our ongoing static-analysis research while reviewing kernel code.

---
Found by inspection; not runtime tested.

 sound/soc/codecs/rt5663.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

index 262d3bba1f3d..2430bce31b3b 100644
@@ -3179,6 +3179,13 @@ static void rt5663_remove(struct snd_soc_component *component)
 {
 	struct rt5663_priv *rt5663 = snd_soc_component_get_drvdata(component);
 
+	/*
+	 * Bound teardown runs this after snd_soc_link_exit().  set_jack()
+	 * there can queue jack_detect_work, which can queue jd_unplug_work.
+	 */
+	cancel_delayed_work_sync(&rt5663->jack_detect_work);
+	cancel_delayed_work_sync(&rt5663->jd_unplug_work);
+
 	regmap_write(rt5663->regmap, RT5663_RESET, 0);
 }
 

@@ -3728,6 +3735,13 @@ static void rt5663_i2c_remove(struct i2c_client *i2c)
 	if (i2c->irq)
 		free_irq(i2c->irq, rt5663);
 
+	/*
+	 * component .remove is skipped when component->card is NULL.
+	 * jack_detect_work can queue jd_unplug_work, so cancel it first.
+	 */
+	cancel_delayed_work_sync(&rt5663->jack_detect_work);
+	cancel_delayed_work_sync(&rt5663->jd_unplug_work);
+
 	regulator_bulk_disable(ARRAY_SIZE(rt5663->supplies), rt5663->supplies);
 }
 

base-commit: 238650ef6c7c7cca08e032527329424c9fbd70e5

2.53.0

This is an extra copy.

The same is true for the probe error handling, if we fail component registration the same situation can happen.