[PATCH 0/6] Fix direct renaming of hashed controls
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
I've noticed that some of mixer controls on my sound card seem to be partially broken on the 6.0 kernel - alsactl wasn't able to find them when restoring the mixer state.
The issue was traced down to the recent addition of hashed controls lookup in commit c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups").
Since that commit it is *not* enough to just directly update the control name field (like some of ALSA drivers were doing). Now the hash entries for the modified control have to be updated too.
This patch set adds a snd_ctl_rename() function that takes care of doing this operation properly for callers that already have the relevant struct snd_kcontrol at hand and hold the control write lock (or simply haven't registered the card yet).
These prerequisites hold true for all the call sites modified.
The core controls change and the emu10k1 patch were runtime tested. Similar patches for other devices were only compile tested.
include/sound/control.h | 1 + sound/core/control.c | 23 +++++++++++++++++++++++ sound/pci/ac97/ac97_codec.c | 32 ++++++++++++++++++++++++-------- sound/pci/ca0106/ca0106_mixer.c | 2 +- sound/pci/emu10k1/emumixer.c | 2 +- sound/pci/hda/patch_realtek.c | 2 +- sound/usb/mixer.c | 2 +- 7 files changed, 52 insertions(+), 12 deletions(-)
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
Add a snd_ctl_rename() function that takes care of updating the control hash entries for callers that already have the relevant struct snd_kcontrol at hand and hold the control write lock (or simply haven't registered the card yet).
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- include/sound/control.h | 1 + sound/core/control.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/include/sound/control.h b/include/sound/control.h index eae443ba79ba5..cc3dcc6cfb0f2 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -138,6 +138,7 @@ int snd_ctl_remove(struct snd_card * card, struct snd_kcontrol * kcontrol); int snd_ctl_replace(struct snd_card *card, struct snd_kcontrol *kcontrol, bool add_on_replace); int snd_ctl_remove_id(struct snd_card * card, struct snd_ctl_elem_id *id); int snd_ctl_rename_id(struct snd_card * card, struct snd_ctl_elem_id *src_id, struct snd_ctl_elem_id *dst_id); +void snd_ctl_rename(struct snd_card *card, struct snd_kcontrol *kctl, const char *name); int snd_ctl_activate_id(struct snd_card *card, struct snd_ctl_elem_id *id, int active); struct snd_kcontrol *snd_ctl_find_numid(struct snd_card * card, unsigned int numid); struct snd_kcontrol *snd_ctl_find_id(struct snd_card * card, struct snd_ctl_elem_id *id); diff --git a/sound/core/control.c b/sound/core/control.c index a7271927d875f..50e7ba66f1876 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -753,6 +753,29 @@ int snd_ctl_rename_id(struct snd_card *card, struct snd_ctl_elem_id *src_id, } EXPORT_SYMBOL(snd_ctl_rename_id);
+/** + * snd_ctl_rename - rename the control on the card + * @card: the card instance + * @kctl: the control to rename + * @name: the new name + * + * Renames the specified control on the card to the new name. + * + * Make sure to take the control write lock - down_write(&card->controls_rwsem). + */ +void snd_ctl_rename(struct snd_card *card, struct snd_kcontrol *kctl, + const char *name) +{ + remove_hash_entries(card, kctl); + + if (strscpy(kctl->id.name, name, sizeof(kctl->id.name)) < 0) + pr_warn("ALSA: Renamed control new name '%s' truncated to '%s'\n", + name, kctl->id.name); + + add_hash_entries(card, kctl); +} +EXPORT_SYMBOL(snd_ctl_rename); + #ifndef CONFIG_SND_CTL_FAST_LOOKUP static struct snd_kcontrol * snd_ctl_find_numid_slow(struct snd_card *card, unsigned int numid)
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
With the recent addition of hashed controls lookup it's not enough to just update the control name field, the hash entries for the modified control have to be updated too.
snd_ctl_rename() takes care of that, so use it instead of directly modifying the control name.
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- sound/usb/mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index a5641956ef102..9105ec623120a 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1631,7 +1631,7 @@ static void check_no_speaker_on_headset(struct snd_kcontrol *kctl, if (!found) return;
- strscpy(kctl->id.name, "Headphone", sizeof(kctl->id.name)); + snd_ctl_rename(card, kctl, "Headphone"); }
static const struct usb_feature_control_info *get_feature_control_info(int control)
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
With the recent addition of hashed controls lookup it's not enough to just update the control name field, the hash entries for the modified control have to be updated too.
snd_ctl_rename() takes care of that, so use it instead of directly modifying the control name.
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- sound/pci/hda/patch_realtek.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 79acd2a2caf20..9945861f02efa 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -2142,7 +2142,7 @@ static void rename_ctl(struct hda_codec *codec, const char *oldname,
kctl = snd_hda_find_mixer_ctl(codec, oldname); if (kctl) - strcpy(kctl->id.name, newname); + snd_ctl_rename(codec->card, kctl, newname); }
static void alc1220_fixup_gb_dual_codecs(struct hda_codec *codec,
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
With the recent addition of hashed controls lookup it's not enough to just update the control name field, the hash entries for the modified control have to be updated too.
snd_ctl_rename() takes care of that, so use it instead of directly modifying the control name.
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- sound/pci/emu10k1/emumixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/emu10k1/emumixer.c b/sound/pci/emu10k1/emumixer.c index e9c0fe3b84461..3c115f8ab96c0 100644 --- a/sound/pci/emu10k1/emumixer.c +++ b/sound/pci/emu10k1/emumixer.c @@ -1767,7 +1767,7 @@ static int rename_ctl(struct snd_card *card, const char *src, const char *dst) { struct snd_kcontrol *kctl = ctl_find(card, src); if (kctl) { - strcpy(kctl->id.name, dst); + snd_ctl_rename(card, kctl, dst); return 0; } return -ENOENT;
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
With the recent addition of hashed controls lookup it's not enough to just update the control name field, the hash entries for the modified control have to be updated too.
snd_ctl_rename() takes care of that, so use it instead of directly modifying the control name.
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- sound/pci/ca0106/ca0106_mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/ca0106/ca0106_mixer.c b/sound/pci/ca0106/ca0106_mixer.c index 05f56015ddd87..f6381c098d4f6 100644 --- a/sound/pci/ca0106/ca0106_mixer.c +++ b/sound/pci/ca0106/ca0106_mixer.c @@ -720,7 +720,7 @@ static int rename_ctl(struct snd_card *card, const char *src, const char *dst) { struct snd_kcontrol *kctl = ctl_find(card, src); if (kctl) { - strcpy(kctl->id.name, dst); + snd_ctl_rename(card, kctl, dst); return 0; } return -ENOENT;
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
With the recent addition of hashed controls lookup it's not enough to just update the control name field, the hash entries for the modified control have to be updated too.
snd_ctl_rename() takes care of that, so use it instead of directly modifying the control name.
While we are at it, check also that the new control name doesn't accidentally overwrite the available buffer space.
Fixes: c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups") Cc: stable@vger.kernel.org Signed-off-by: Maciej S. Szmigiero maciej.szmigiero@oracle.com --- sound/pci/ac97/ac97_codec.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c index ceead55f13ab1..ff685321f1a11 100644 --- a/sound/pci/ac97/ac97_codec.c +++ b/sound/pci/ac97/ac97_codec.c @@ -2656,11 +2656,18 @@ EXPORT_SYMBOL(snd_ac97_resume); */ static void set_ctl_name(char *dst, const char *src, const char *suffix) { - if (suffix) - sprintf(dst, "%s %s", src, suffix); - else - strcpy(dst, src); -} + const size_t msize = SNDRV_CTL_ELEM_ID_NAME_MAXLEN; + + if (suffix) { + if (snprintf(dst, msize, "%s %s", src, suffix) >= msize) + pr_warn("ALSA: AC97 control name '%s %s' truncated to '%s'\n", + src, suffix, dst); + } else { + if (strscpy(dst, src, msize) < 0) + pr_warn("ALSA: AC97 control name '%s' truncated to '%s'\n", + src, dst); + } +}
/* remove the control with the given name and optional suffix */ static int snd_ac97_remove_ctl(struct snd_ac97 *ac97, const char *name, @@ -2687,8 +2694,11 @@ static int snd_ac97_rename_ctl(struct snd_ac97 *ac97, const char *src, const char *dst, const char *suffix) { struct snd_kcontrol *kctl = ctl_find(ac97, src, suffix); + char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; + if (kctl) { - set_ctl_name(kctl->id.name, dst, suffix); + set_ctl_name(name, dst, suffix); + snd_ctl_rename(ac97->bus->card, kctl, name); return 0; } return -ENOENT; @@ -2707,11 +2717,17 @@ static int snd_ac97_swap_ctl(struct snd_ac97 *ac97, const char *s1, const char *s2, const char *suffix) { struct snd_kcontrol *kctl1, *kctl2; + char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN]; + kctl1 = ctl_find(ac97, s1, suffix); kctl2 = ctl_find(ac97, s2, suffix); if (kctl1 && kctl2) { - set_ctl_name(kctl1->id.name, s2, suffix); - set_ctl_name(kctl2->id.name, s1, suffix); + set_ctl_name(name, s2, suffix); + snd_ctl_rename(ac97->bus->card, kctl1, name); + + set_ctl_name(name, s1, suffix); + snd_ctl_rename(ac97->bus->card, kctl2, name); + return 0; } return -ENOENT;
On Thu, 20 Oct 2022 22:46:20 +0200, Maciej S. Szmigiero wrote:
From: "Maciej S. Szmigiero" maciej.szmigiero@oracle.com
I've noticed that some of mixer controls on my sound card seem to be partially broken on the 6.0 kernel - alsactl wasn't able to find them when restoring the mixer state.
The issue was traced down to the recent addition of hashed controls lookup in commit c27e1efb61c5 ("ALSA: control: Use xarray for faster lookups").
Since that commit it is *not* enough to just directly update the control name field (like some of ALSA drivers were doing). Now the hash entries for the modified control have to be updated too.
This patch set adds a snd_ctl_rename() function that takes care of doing this operation properly for callers that already have the relevant struct snd_kcontrol at hand and hold the control write lock (or simply haven't registered the card yet).
These prerequisites hold true for all the call sites modified.
The core controls change and the emu10k1 patch were runtime tested. Similar patches for other devices were only compile tested.
Good catch! Applied all patches now.
thanks,
Takashi
participants (2)
-
Maciej S. Szmigiero -
Takashi Iwai