Re: [PATCH] ALSA: hda: Do not unset preset when cleaning up codec

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 1/17/23 09:47, 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

very hard to follow.
Is there a spurious 'what' to be removed?
Or is there missing text?
Please consider rewording with simpler sentences.

> 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.
> 
> [1]: https://lore.kernel.org/alsa-devel/20220706120230.427296-2-cezary.rojewski@intel.com/
> 
>  sound/pci/hda/hda_codec.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index edd653ece70d..ac1cc7c5290e 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -795,7 +795,6 @@ void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
>  	snd_array_free(&codec->cvt_setups);
>  	snd_array_free(&codec->spdif_out);
>  	snd_array_free(&codec->verbs);
> -	codec->preset = NULL;
>  	codec->follower_dig_outs = NULL;
>  	codec->spdif_status_reset = 0;
>  	snd_array_free(&codec->mixers);