ALSA: control: Take controls_rwsem lock in snd_ctl_remove()

So far, snd_ctl_remove() requires its caller to take
card->controls_rwsem manually before the call for avoiding possible
races.  However, many callers don't care and miss the locking.

Basically it's cumbersome and error-prone to enforce it to each
caller.  Moreover, card->controls_rwsem is a field that should be used
only by internal or proper helpers, and it's not to be touched at
random external places.

This patch is an attempt to make those calls more consistent: now
snd_ctl_remove() takes the card->controls_rwsem internally, just like
other API functions for kctls.  Since a few callers already take the
controls_rwsem locks, the patch removes those locks at the same time,
too.

Link: https://lore.kernel.org/r/20230718141304.1032-5-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/control.c b/sound/core/control.c
index a41d19c..9c93335 100644 (file)
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -39,6 +39,9 @@ static LIST_HEAD(snd_control_compat_ioctls);
 #endif
 static struct snd_ctl_layer_ops *snd_ctl_layer;
 
+static int snd_ctl_remove_locked(struct snd_card *card,
+                                struct snd_kcontrol *kcontrol);
+
 static int snd_ctl_open(struct inode *inode, struct file *file)
 {
        unsigned long flags;
@@ -483,7 +486,7 @@ static int __snd_ctl_add_replace(struct snd_card *card,
                        return -EBUSY;
                }
 
-               err = snd_ctl_remove(card, old);
+               err = snd_ctl_remove_locked(card, old);
                if (err < 0)
                        return err;
        }
@@ -589,20 +592,32 @@ static int __snd_ctl_remove(struct snd_card *card,
        return 0;
 }
 
+static inline int snd_ctl_remove_locked(struct snd_card *card,
+                                       struct snd_kcontrol *kcontrol)
+{
+       return __snd_ctl_remove(card, kcontrol, true);
+}
+
 /**
  * snd_ctl_remove - remove the control from the card and release it
  * @card: the card instance
  * @kcontrol: the control instance to remove
  *
  * Removes the control from the card and then releases the instance.
- * You don't need to call snd_ctl_free_one(). You must be in
- * the write lock - down_write(&card->controls_rwsem).
+ * You don't need to call snd_ctl_free_one().
  *
  * Return: 0 if successful, or a negative error code on failure.
+ *
+ * Note that this function takes card->controls_rwsem lock internally.
  */
 int snd_ctl_remove(struct snd_card *card, struct snd_kcontrol *kcontrol)
 {
-       return __snd_ctl_remove(card, kcontrol, true);
+       int ret;
+
+       down_write(&card->controls_rwsem);
+       ret = snd_ctl_remove_locked(card, kcontrol);
+       up_write(&card->controls_rwsem);
+       return ret;
 }
 EXPORT_SYMBOL(snd_ctl_remove);
 
@@ -627,7 +642,7 @@ int snd_ctl_remove_id(struct snd_card *card, struct snd_ctl_elem_id *id)
                up_write(&card->controls_rwsem);
                return -ENOENT;
        }
-       ret = snd_ctl_remove(card, kctl);
+       ret = snd_ctl_remove_locked(card, kctl);
        up_write(&card->controls_rwsem);
        return ret;
 }
@@ -665,7 +680,7 @@ static int snd_ctl_remove_user_ctl(struct snd_ctl_file * file,
                        ret = -EBUSY;
                        goto error;
                }
-       ret = snd_ctl_remove(card, kctl);
+       ret = snd_ctl_remove_locked(card, kctl);
 error:
        up_write(&card->controls_rwsem);
        return ret;

diff --git a/sound/core/jack.c b/sound/core/jack.c
index 03d155e..e0f034e 100644 (file)
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -66,12 +66,10 @@ static int snd_jack_dev_free(struct snd_device *device)
        struct snd_card *card = device->card;
        struct snd_jack_kctl *jack_kctl, *tmp_jack_kctl;
 
-       down_write(&card->controls_rwsem);
        list_for_each_entry_safe(jack_kctl, tmp_jack_kctl, &jack->kctl_list, list) {
                list_del_init(&jack_kctl->list);
                snd_ctl_remove(card, jack_kctl->kctl);
        }
-       up_write(&card->controls_rwsem);
 
        if (jack->private_free)
                jack->private_free(jack);

diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 9d95e37..1c0bb3a 100644 (file)
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -814,9 +814,7 @@ static void free_chmap(struct snd_pcm_str *pstr)
        if (pstr->chmap_kctl) {
                struct snd_card *card = pstr->pcm->card;
 
-               down_write(&card->controls_rwsem);
                snd_ctl_remove(card, pstr->chmap_kctl);
-               up_write(&card->controls_rwsem);
                pstr->chmap_kctl = NULL;
        }
 }

diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
index e020296..a640577 100644 (file)
--- a/sound/isa/sb/emu8000.c
+++ b/sound/isa/sb/emu8000.c
@@ -1040,10 +1040,8 @@ snd_emu8000_create_mixer(struct snd_card *card, struct snd_emu8000 *emu)
 
 __error:
        for (i = 0; i < EMU8000_NUM_CONTROLS; i++) {
-               down_write(&card->controls_rwsem);
                if (emu->controls[i])
                        snd_ctl_remove(card, emu->controls[i]);
-               up_write(&card->controls_rwsem);
        }
        return err;
 }

diff --git a/sound/isa/sb/sb16_csp.c b/sound/isa/sb/sb16_csp.c
index 7ad8c5f..8d83570 100644 (file)
--- a/sound/isa/sb/sb16_csp.c
+++ b/sound/isa/sb/sb16_csp.c
@@ -1080,7 +1080,6 @@ static void snd_sb_qsound_destroy(struct snd_sb_csp * p)
 
        card = p->chip->card;   
        
-       down_write(&card->controls_rwsem);
        if (p->qsound_switch) {
                snd_ctl_remove(card, p->qsound_switch);
                p->qsound_switch = NULL;
@@ -1089,7 +1088,6 @@ static void snd_sb_qsound_destroy(struct snd_sb_csp * p)
                snd_ctl_remove(card, p->qsound_space);
                p->qsound_space = NULL;
        }
-       up_write(&card->controls_rwsem);
 
        /* cancel pending transfer of QSound parameters */
        spin_lock_irqsave (&p->q_lock, flags);

diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
index 9904bcf..70c8252 100644 (file)
--- a/sound/pci/emu10k1/emufx.c
+++ b/sound/pci/emu10k1/emufx.c
@@ -977,11 +977,9 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu,
                                       in_kernel);
                if (err < 0)
                        return err;
-               down_write(&card->controls_rwsem);
                ctl = snd_emu10k1_look_for_ctl(emu, &id);
                if (ctl)
                        snd_ctl_remove(card, ctl->kcontrol);
-               up_write(&card->controls_rwsem);
        }
        return 0;
 }

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index bd19f92..33af707 100644 (file)
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1769,10 +1769,8 @@ void snd_hda_ctls_clear(struct hda_codec *codec)
        int i;
        struct hda_nid_item *items = codec->mixers.list;
 
-       down_write(&codec->card->controls_rwsem);
        for (i = 0; i < codec->mixers.used; i++)
                snd_ctl_remove(codec->card, items[i].kctl);
-       up_write(&codec->card->controls_rwsem);
        snd_array_free(&codec->mixers);
        snd_array_free(&codec->nids);
 }

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 8add361..03ec3c5 100644 (file)
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -2564,7 +2564,6 @@ EXPORT_SYMBOL_GPL(snd_soc_tplg_component_load);
 /* remove dynamic controls from the component driver */
 int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
 {
-       struct snd_card *card = comp->card->snd_card;
        struct snd_soc_dobj *dobj, *next_dobj;
        int pass;
 
@@ -2572,7 +2571,6 @@ int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
        for (pass = SOC_TPLG_PASS_END; pass >= SOC_TPLG_PASS_START; pass--) {
 
                /* remove mixer controls */
-               down_write(&card->controls_rwsem);
                list_for_each_entry_safe(dobj, next_dobj, &comp->dobj_list,
                        list) {
 
@@ -2607,7 +2605,6 @@ int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
                                break;
                        }
                }
-               up_write(&card->controls_rwsem);
        }
 
        /* let caller know if FW can be freed when no objects are left */