Re: [PATCH 6/9] ALSA: hda: Fix page fault in snd_hda_codec_shutdown()

From: Takashi Iwai <tiwai@suse.de>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: "Cezary Rojewski" <cezary.rojewski@intel.com>,
	"Kai Vehmanen" <kai.vehmanen@linux.intel.com>,
	"Bard Liao" <yung-chuan.liao@linux.intel.com>,
	alsa-devel@alsa-project.org, tiwai@suse.com, hdegoede@redhat.com,
	broonie@kernel.org,
	"Ranjani Sridharan" <ranjani.sridharan@linux.intel.com>,
	amadeuszx.slawinski@linux.intel.com,
	"Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>
Subject: Re: [PATCH 6/9] ALSA: hda: Fix page fault in snd_hda_codec_shutdown()
Date: Fri, 15 Jul 2022 20:23:49 +0200	[thread overview]
Message-ID: <874jziute2.wl-tiwai@suse.de> (raw)
In-Reply-To: <3c40df55-3aee-1e08-493b-7b30cd84dc00@linux.intel.com>

On Fri, 15 Jul 2022 20:16:14 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 7/6/22 07:02, Cezary Rojewski wrote:
> > If early probe of HDAudio bus driver fails e.g.: due to missing
> > firmware file, snd_hda_codec_shutdown() ends in manipulating
> > uninitialized codec->pcm_list_head causing page fault.
> > 
> > Iinitialization of HDAudio codec in ASoC is split in two:
> > - snd_hda_codec_device_init()
> > - snd_hda_codec_device_new()
> > 
> > snd_hda_codec_device_init() is called during probe_codecs() by HDAudio
> > bus driver while snd_hda_codec_device_new() is called by
> > codec-component's ->probe(). The second call will not happen until all
> > components required by related sound card are present within the ASoC
> > framework. With firmware failing to load during the PCI's deferred
> > initialization i.e.: probe_work(), no platform components are ever
> > registered. HDAudio codec enumeration is done at that point though, so
> > the codec components became registered to ASoC framework, calling
> > snd_hda_codec_device_init() in the process.
> > 
> > Now, during platform reboot snd_hda_codec_shutdown() is called for every
> > codec found on the HDAudio bus causing oops if any of them has not
> > completed both of their initialization steps. Relocating field
> > initialization fixes the issue.
> > 
> > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> 
> This patch causes an across-the-board regression on all SOF platforms
> using an HDaudio or iDISP codec. Only 'nocodec' platforms are
> unaffected, see results at
> https://sof-ci.01.org/linuxpr/PR3763/build394/devicetest/
> 
> Reverting this commit seems to fix the issue.
> 
> Upstream merge:
> https://github.com/thesofproject/linux/pull/3763
> 
> Issue and bisect results
> https://github.com/thesofproject/linux/issues/3764
> 
> I don't know what this was supposed to fix on the shutdown path but it's
> clearly having side effects on the probe/init path.

Yeah, obviously the patch ignores the fact that hdac_hda does
initialize the HD-audio codec without snd_hda_codec_device_init().

I'm going to revert the commit.

thanks,

Takashi