On Tue, 17 Jan 2023 16:47:34 +0100, Cezary Rojewski wrote: > > Several functions that take part in codec's initialization and removal > are re-used by ASoC codec drivers implementations. Drivers mimic the > behavior of hda_codec_driver_probe/remove() found in > sound/pci/hda/hda_bind.c with their component->probe/remove() instead. > > One of the reasons for that is the expectation of > snd_hda_codec_device_new() to receive a valid struct snd_card pointer > what cannot be fulfilled on ASoC side until a card is attempted to be > bound and its component probing is triggered. > > As ASoC sound card may be unbound without codec device being actually > removed from the system, unsetting ->preset in > snd_hda_codec_cleanup_for_unbind() interferes with module unload -> load > scenario causing null-ptr-deref. Preset is assigned only once, during > device/driver matching whereas ASoC codec driver's module reloading may > occur several times throughout the lifetime of an audio stack. > > Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx> > --- > > 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? thanks, Takashi
