From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: Cezary Rojewski <cezary.rojewski@intel.com>,
alsa-devel@alsa-project.org, tiwai@suse.com
Cc: "Kai Vehmanen" <kai.vehmanen@linux.intel.com>,
"Bard Liao" <yung-chuan.liao@linux.intel.com>,
"Ranjani Sridharan" <ranjani.sridharan@linux.intel.com>,
hdegoede@redhat.com, broonie@kernel.org,
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 13:16:14 -0500 [thread overview]
Message-ID: <3c40df55-3aee-1e08-493b-7b30cd84dc00@linux.intel.com> (raw)
In-Reply-To: <20220706120230.427296-7-cezary.rojewski@intel.com>
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.
> ---
> sound/pci/hda/hda_codec.c | 41 +++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 21 deletions(-)
>
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index 75e85bf58681..677d0a78f19c 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -930,8 +930,28 @@ snd_hda_codec_device_init(struct hda_bus *bus, unsigned int codec_addr,
> }
>
> codec->bus = bus;
> + codec->depop_delay = -1;
> + codec->fixup_id = HDA_FIXUP_ID_NOT_SET;
> + codec->core.dev.release = snd_hda_codec_dev_release;
> + codec->core.exec_verb = codec_exec_verb;
> codec->core.type = HDA_DEV_LEGACY;
>
> + mutex_init(&codec->spdif_mutex);
> + mutex_init(&codec->control_mutex);
> + snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
> + snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
> + snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
> + snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
> + snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
> + snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
> + snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
> + snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
> + INIT_LIST_HEAD(&codec->conn_list);
> + INIT_LIST_HEAD(&codec->pcm_list_head);
> + INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
> + refcount_set(&codec->pcm_ref, 1);
> + init_waitqueue_head(&codec->remove_sleep);
> +
> return codec;
> }
> EXPORT_SYMBOL_GPL(snd_hda_codec_device_init);
> @@ -984,29 +1004,8 @@ int snd_hda_codec_device_new(struct hda_bus *bus, struct snd_card *card,
> if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
> return -EINVAL;
>
> - codec->core.dev.release = snd_hda_codec_dev_release;
> - codec->core.exec_verb = codec_exec_verb;
> -
> codec->card = card;
> codec->addr = codec_addr;
> - mutex_init(&codec->spdif_mutex);
> - mutex_init(&codec->control_mutex);
> - snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
> - snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
> - snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
> - snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
> - snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
> - snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
> - snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
> - snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
> - INIT_LIST_HEAD(&codec->conn_list);
> - INIT_LIST_HEAD(&codec->pcm_list_head);
> - refcount_set(&codec->pcm_ref, 1);
> - init_waitqueue_head(&codec->remove_sleep);
> -
> - INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
> - codec->depop_delay = -1;
> - codec->fixup_id = HDA_FIXUP_ID_NOT_SET;
>
> #ifdef CONFIG_PM
> codec->power_jiffies = jiffies;
next prev parent reply other threads:[~2022-07-15 18:17 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 12:02 [PATCH 0/9] ALSA: hda: Codec-reload bug fixes and cleanups Cezary Rojewski
2022-07-06 12:02 ` [PATCH 1/9] ALSA: hda: Do not unset preset when cleaning up codec Cezary Rojewski
2022-07-09 16:34 ` Takashi Iwai
2022-07-11 8:25 ` Cezary Rojewski
2022-07-11 14:12 ` Takashi Iwai
2022-07-12 9:42 ` Cezary Rojewski
2022-07-12 10:46 ` Takashi Iwai
2022-07-12 10:58 ` Cezary Rojewski
2022-07-15 14:55 ` Takashi Iwai
2023-01-17 14:45 ` Cezary Rojewski
2023-01-17 14:51 ` Takashi Iwai
2022-07-06 12:02 ` [PATCH 2/9] ALSA: hda: Fix null-ptr-deref when i915 fails and hdmi is denylisted Cezary Rojewski
2022-07-06 12:02 ` [PATCH 3/9] ALSA: hda: Make device usage_count consistent across subsequent probing Cezary Rojewski
2022-07-06 12:02 ` [PATCH 4/9] ALSA: hda: Fix put_device() inconsistency in error path Cezary Rojewski
2022-07-06 12:02 ` [PATCH 5/9] ALSA: hda: Skip event processing for unregistered codecs Cezary Rojewski
2022-07-09 16:47 ` Takashi Iwai
2022-07-15 14:27 ` Takashi Iwai
2022-07-06 12:02 ` [PATCH 6/9] ALSA: hda: Fix page fault in snd_hda_codec_shutdown() Cezary Rojewski
2022-07-15 18:16 ` Pierre-Louis Bossart [this message]
2022-07-15 18:23 ` Takashi Iwai
2022-07-17 10:05 ` Cezary Rojewski
2022-07-06 12:02 ` [PATCH 7/9] ALSA: hda: Reset all SIE bits in INTCTL Cezary Rojewski
2022-07-06 12:02 ` [PATCH 8/9] ALSA: hda: Remove unused macro definition Cezary Rojewski
2022-07-06 12:02 ` [PATCH 9/9] ALSA: hda/realtek: Remove redundant init_hook() in alc_default_init() Cezary Rojewski
2022-07-09 16:46 ` Takashi Iwai
2022-07-11 8:12 ` Cezary Rojewski
2022-07-09 16:50 ` [PATCH 0/9] ALSA: hda: Codec-reload bug fixes and cleanups Takashi Iwai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3c40df55-3aee-1e08-493b-7b30cd84dc00@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=amadeuszx.slawinski@linux.intel.com \
--cc=broonie@kernel.org \
--cc=cezary.rojewski@intel.com \
--cc=hdegoede@redhat.com \
--cc=kai.vehmanen@linux.intel.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=tiwai@suse.com \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.