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@xxxxxxxxxxxxxxxxxxxxx> > 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 >