ASoC: soc-card: Fix missing locking in snd_soc_card_get_kcontrol()

snd_soc_card_get_kcontrol() must be holding a read lock on
card->controls_rwsem while walking the controls list.

Compare with snd_ctl_find_numid().

The existing function is renamed snd_soc_card_get_kcontrol_locked()
so that it can be called from contexts that are already holding
card->controls_rwsem (for example, control get/put functions).

There are few direct or indirect callers of
snd_soc_card_get_kcontrol(), and most are safe. Three require
changes, which have been included in this patch:

codecs/cs35l45.c:
  cs35l45_activate_ctl() is called from a control put() function so
  is changed to call snd_soc_card_get_kcontrol_locked().

codecs/cs35l56.c:
  cs35l56_sync_asp1_mixer_widgets_with_firmware() is called from
  control get()/put() functions so is changed to call
  snd_soc_card_get_kcontrol_locked().

fsl/fsl_xcvr.c:
  fsl_xcvr_activate_ctl() is called from three places, one of which
  already holds card->controls_rwsem:
  1. fsl_xcvr_mode_put(), a control put function, which will
     already be holding card->controls_rwsem.
  2. fsl_xcvr_startup(), a DAI startup function.
  3. fsl_xcvr_shutdown(), a DAI shutdown function.

  To fix this, fsl_xcvr_activate_ctl() has been changed to call
  snd_soc_card_get_kcontrol_locked() so that it is safe to call
  directly from fsl_xcvr_mode_put().
  The fsl_xcvr_startup() and fsl_xcvr_shutdown() functions have been
  changed to take a read lock on card->controls_rsem() around calls
  to fsl_xcvr_activate_ctl(). While this is not very elegant, it
  keeps the change small, to avoid this patch creating a large
  collateral churn in fsl/fsl_xcvr.c.

Analysis of other callers of snd_soc_card_get_kcontrol() is that
they do not need any changes, they are not holding card->controls_rwsem
when they call snd_soc_card_get_kcontrol().

Direct callers of snd_soc_card_get_kcontrol():
  fsl/fsl_spdif.c: fsl_spdif_dai_probe() - DAI probe function
  fsl/fsl_micfil.c: voice_detected_fn() - IRQ handler

Indirect callers via soc_component_notify_control():
  codecs/cs42l43: cs42l43_mic_shutter() - IRQ handler
  codecs/cs42l43: cs42l43_spk_shutter() - IRQ handler
  codecs/ak4118.c: ak4118_irq_handler() - IRQ handler
  codecs/wm_adsp.c: wm_adsp_write_ctl() - not currently used

Indirect callers via snd_soc_limit_volume():
  qcom/sc8280xp.c: sc8280xp_snd_init() - DAIlink init function
  ti/rx51.c: rx51_aic34_init() - DAI init function

I don't have hardware to test the fsl/*, qcom/sc828xp.c, ti/rx51.c
and ak4118.c changes.

Backport note:
The fsl/, qcom/, cs35l45, cs35l56 and cs42l43 callers were added
since the Fixes commit so won't all be present on older kernels.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Fixes: 209c6cdfd283 ("ASoC: soc-card: move snd_soc_card_get_kcontrol() to soc-card")
Link: https://lore.kernel.org/r/20240221123710.690224-1-rf@opensource.cirrus.com
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/include/sound/soc-card.h b/include/sound/soc-card.h
index ecc02e955279fdfa3f10d116eeb5a2d7271cc91c..1f4c39922d825035be15ba38e7ac6d112b36c457 100644 (file)
--- a/include/sound/soc-card.h
+++ b/include/sound/soc-card.h
@@ -30,6 +30,8 @@ static inline void snd_soc_card_mutex_unlock(struct snd_soc_card *card)
 
 struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
                                               const char *name);
+struct snd_kcontrol *snd_soc_card_get_kcontrol_locked(struct snd_soc_card *soc_card,
+                                                     const char *name);
 int snd_soc_card_jack_new(struct snd_soc_card *card, const char *id, int type,
                          struct snd_soc_jack *jack);
 int snd_soc_card_jack_new_pins(struct snd_soc_card *card, const char *id,

diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c
index 44c221745c3b255c7e69a4ab092ad09581a9b1f9..2392c6effed857c32326ee032f776dcc577ed0fb 100644 (file)
--- a/sound/soc/codecs/cs35l45.c
+++ b/sound/soc/codecs/cs35l45.c
@@ -184,7 +184,7 @@ static int cs35l45_activate_ctl(struct snd_soc_component *component,
        else
                snprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "%s", ctl_name);
 
-       kcontrol = snd_soc_card_get_kcontrol(component->card, name);
+       kcontrol = snd_soc_card_get_kcontrol_locked(component->card, name);
        if (!kcontrol) {
                dev_err(component->dev, "Can't find kcontrol %s\n", name);
                return -EINVAL;

diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 2c1313e34cce4969ca393ef774e305a4d18198b5..6dd0319bc843cf5d1740222e57c380881b4b5b05 100644 (file)
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -114,7 +114,7 @@ static int cs35l56_sync_asp1_mixer_widgets_with_firmware(struct cs35l56_private
                        name = full_name;
                }
 
-               kcontrol = snd_soc_card_get_kcontrol(dapm->card, name);
+               kcontrol = snd_soc_card_get_kcontrol_locked(dapm->card, name);
                if (!kcontrol) {
                        dev_warn(cs35l56->base.dev, "Could not find control %s\n", name);
                        continue;

diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index f0fb33d719c25135722014f9763c65df3289ed7e..c46f64557a7ffd268716e852dfe4411251da08cc 100644 (file)
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -174,7 +174,9 @@ static int fsl_xcvr_activate_ctl(struct snd_soc_dai *dai, const char *name,
        struct snd_kcontrol *kctl;
        bool enabled;
 
-       kctl = snd_soc_card_get_kcontrol(card, name);
+       lockdep_assert_held(&card->snd_card->controls_rwsem);
+
+       kctl = snd_soc_card_get_kcontrol_locked(card, name);
        if (kctl == NULL)
                return -ENOENT;
 
@@ -576,10 +578,14 @@ static int fsl_xcvr_startup(struct snd_pcm_substream *substream,
        xcvr->streams |= BIT(substream->stream);
 
        if (!xcvr->soc_data->spdif_only) {
+               struct snd_soc_card *card = dai->component->card;
+
                /* Disable XCVR controls if there is stream started */
+               down_read(&card->snd_card->controls_rwsem);
                fsl_xcvr_activate_ctl(dai, fsl_xcvr_mode_kctl.name, false);
                fsl_xcvr_activate_ctl(dai, fsl_xcvr_arc_mode_kctl.name, false);
                fsl_xcvr_activate_ctl(dai, fsl_xcvr_earc_capds_kctl.name, false);
+               up_read(&card->snd_card->controls_rwsem);
        }
 
        return 0;
@@ -598,11 +604,15 @@ static void fsl_xcvr_shutdown(struct snd_pcm_substream *substream,
        /* Enable XCVR controls if there is no stream started */
        if (!xcvr->streams) {
                if (!xcvr->soc_data->spdif_only) {
+                       struct snd_soc_card *card = dai->component->card;
+
+                       down_read(&card->snd_card->controls_rwsem);
                        fsl_xcvr_activate_ctl(dai, fsl_xcvr_mode_kctl.name, true);
                        fsl_xcvr_activate_ctl(dai, fsl_xcvr_arc_mode_kctl.name,
                                                (xcvr->mode == FSL_XCVR_MODE_ARC));
                        fsl_xcvr_activate_ctl(dai, fsl_xcvr_earc_capds_kctl.name,
                                                (xcvr->mode == FSL_XCVR_MODE_EARC));
+                       up_read(&card->snd_card->controls_rwsem);
                }
                ret = regmap_update_bits(xcvr->regmap, FSL_XCVR_EXT_IER0,
                                         FSL_XCVR_IRQ_EARC_ALL, 0);

diff --git a/sound/soc/soc-card.c b/sound/soc/soc-card.c
index 285ab4c9c7168314ae34bead44a5229bc5d8b96b..8a2f163da6bc9e8e61fc1f196f3b89556815cafe 100644 (file)
--- a/sound/soc/soc-card.c
+++ b/sound/soc/soc-card.c
@@ -5,6 +5,9 @@
 // Copyright (C) 2019 Renesas Electronics Corp.
 // Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
 //
+
+#include <linux/lockdep.h>
+#include <linux/rwsem.h>
 #include <sound/soc.h>
 #include <sound/jack.h>
 
@@ -26,12 +29,15 @@ static inline int _soc_card_ret(struct snd_soc_card *card,
        return ret;
 }
 
-struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
-                                              const char *name)
+struct snd_kcontrol *snd_soc_card_get_kcontrol_locked(struct snd_soc_card *soc_card,
+                                                     const char *name)
 {
        struct snd_card *card = soc_card->snd_card;
        struct snd_kcontrol *kctl;
 
+       /* must be held read or write */
+       lockdep_assert_held(&card->controls_rwsem);
+
        if (unlikely(!name))
                return NULL;
 
@@ -40,6 +46,20 @@ struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
                        return kctl;
        return NULL;
 }
+EXPORT_SYMBOL_GPL(snd_soc_card_get_kcontrol_locked);
+
+struct snd_kcontrol *snd_soc_card_get_kcontrol(struct snd_soc_card *soc_card,
+                                              const char *name)
+{
+       struct snd_card *card = soc_card->snd_card;
+       struct snd_kcontrol *kctl;
+
+       down_read(&card->controls_rwsem);
+       kctl = snd_soc_card_get_kcontrol_locked(soc_card, name);
+       up_read(&card->controls_rwsem);
+
+       return kctl;
+}
 EXPORT_SYMBOL_GPL(snd_soc_card_get_kcontrol);
 
 static int jack_new(struct snd_soc_card *card, const char *id, int type,