[PATCH] ALSA: hda: Do not unset preset when cleaning up codec
Takashi Iwai
tiwai at suse.de
Wed Jan 18 13:32:33 CET 2023
On Wed, 18 Jan 2023 13:04:05 +0100,
Cezary Rojewski wrote:
>
> On 2023-01-17 5:01 PM, Takashi Iwai wrote:
>
> ...
>
> >> This is a continuation of a discussion that begun in the middle of 2022
> >> [1] and was part of a larger series addressing several HDAudio topics.
> >>
> >> Single rmmod on ASoC's codec driver module is enough to cause a panic.
> >> Given our results, no regression shows up with modprobe/rmmod on
> >> snd_hda_intel side with this patch applied.
> >
> > I think one possible regression by this change would be the case you
> > reload another codec driver. With keeping codec->preset, it's still
> > thought as if already matched, and a wrong one could be used.
> >
> > And, this would be nothing but a leak of the possibly freed address.
> > After hda_codec_driver_remove(), card->preset may point to an already
> > freed address.
> >
> > So, just removing isn't right. It has to be cleared somewhere
> > instead, e.g. in hda_bind.c.
> >
> > But, one thing I'm still concerned is that your comment about the call
> > without the card binding. Do you mean that the
> > snd_hda_codec_cleanup_for_unbind() may be called even if codec->card
> > isn't set?
>
> One proposition would be to add line:
> codec->preset = NULL;
>
> after both invocations of snd_hda_codec_cleanup_for_unbind() in
> hda_codec_driver_probe/remove() in sound/pci/hda/hda_bind.c.
Yes, that's what I meant, too.
> In regard to your last question - no, cleanup is only called either in
> component->probe()'s error path or in component->remove() once
> snd_hda_codec_device_new() succeeds and thus, codec->card points to a
> valid sound card.
>
> What I meant by my comment, is that removal of any components of an
> ASoC sound card will cause all other components to have their
> ->remove() op called too. Let's say I unload snd_soc_avs_hdaudio
> module without unloading snd_soc_avs. As bus (snd_soc_avs) owns the
> codecs, its platform component->remove() gets called right after codec
> component->remove() but the actual devices are still present in the
> system even with the sound card module (snd_soc_avs_hdaudio) unloaded.
OK, then the snd_hda_codec object itself is kept bound on the bus,
hence the call of snd_hda_codec_cleanup_for_unbind() is somehow
inappropriate. The function could be renamed for avoid
misunderstanding.
thanks,
Takashi
More information about the Alsa-devel
mailing list