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@intel.com
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