On Fri, 03 May 2024 16:49:20 +0200, Richard Fitzgerald wrote:
hda_cs_dsp_control_remove() must remove the ALSA control when deleting all the infrastructure for handling the control.
Without this it is possible for ALSA controls to be left in the Soundcard after the amp driver module has been unloaded. So the get/set callbacks point to code that no longer exists.
Signed-off-by: Richard Fitzgerald rf@opensource.cirrus.com Fixes: 3233b978af23 ("ALSA: hda: hda_cs_dsp_ctl: Add Library to support CS_DSP ALSA controls")
Note: it would be better to use the control private_free to do the cleanup, and that is my plan long-term. But that is a larger change to the code.
I like to keep bugfix patches as simple as possible so they are low-risk and easy to cherry-pick into older kernels. So this patch fixes the bug. Sometime I will send a patch for future kernel versions that reworks the cleanup to use private_free.
I also like to keep as simple as possible :)
One slight concern is whether cs_dsp kctls can be deleted at the snd_card removal (disconnect) before this function gets called. That is, snd_card_free() of the main card may delete all associated kctls, and may this function be called afterwards? If yes, this change would lead to a UAF.
The structure is so complex and I can't follow immediately, unfortunately...
thanks,
Takashi
sound/pci/hda/hda_cs_dsp_ctl.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/pci/hda/hda_cs_dsp_ctl.c b/sound/pci/hda/hda_cs_dsp_ctl.c index 463ca06036bf..a42653d3473d 100644 --- a/sound/pci/hda/hda_cs_dsp_ctl.c +++ b/sound/pci/hda/hda_cs_dsp_ctl.c @@ -203,6 +203,10 @@ void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl) { struct hda_cs_dsp_coeff_ctl *ctl = cs_ctl->priv;
- /* Only public firmware controls will have an associated kcontrol */
- if (ctl && ctl->kctl)
snd_ctl_remove(ctl->card, ctl->kctl);- kfree(ctl);
} EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_remove, SND_HDA_CS_DSP_CONTROLS); -- 2.39.2