Re: [alsa-devel] 3.6-rc Cirrus HDA reconfig oops...
At Sun, 9 Sep 2012 13:02:16 +0800, Daniel J Blueman wrote:
With a non-specialised Cirrus HDA codec, it's readily possible to cause a kernel oops as root with 3.6-rc kernels [1].
patch_cirrus.c:
static int cs_init(struct hda_codec *codec) { struct cs_spec *spec = codec->spec;
/* init_verb sequence for C0/C1/C2 errata*/ snd_hda_sequence_write(codec, cs_errata_init_verbs);
snd_hda_sequence_write(codec, cs_coef_init_verbs);
if (spec->gpio_mask) { snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK, spec->gpio_mask);
Here, spec is NULL, so loading spec->gpio_mask causes a fatal pagefault at address 0x180 (ie the offset of gpio_mask in the struct). I was going to prepare a patch to guard for this, but since spec is expected in so many places, there is a likely a behavioural issue preventing a spec struct being generated.
Let me know for further testing and debug.
It's because of calling a leftover callback. The patch below should fix the issue. I'll queue it up to for-linus branch.
thanks,
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix Oops at codec reset/reconfig
snd_hda_codec_reset() calls restore_pincfgs() where the codec is powered up again, which eventually tries to resume and initialize via the callbacks of the codec. However, it's the place just after codec free callback, thus no codec callbacks should be called after that. On a codec like CS4206, it results in Oops due to the access in init callback.
This patch fixes the issue by clearing the codec callbacks properly after freeing codec.
Reported-by: Daniel J Blueman daniel@quora.org Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index f25c24c..aa4cbca 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2353,6 +2353,7 @@ int snd_hda_codec_reset(struct hda_codec *codec) } if (codec->patch_ops.free) codec->patch_ops.free(codec); + memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); snd_hda_jack_tbl_clear(codec); codec->proc_widget_hook = NULL; codec->spec = NULL;
At Mon, 10 Sep 2012 09:46:43 +0200, Takashi Iwai wrote:
At Sun, 9 Sep 2012 13:02:16 +0800, Daniel J Blueman wrote:
With a non-specialised Cirrus HDA codec, it's readily possible to cause a kernel oops as root with 3.6-rc kernels [1].
patch_cirrus.c:
static int cs_init(struct hda_codec *codec) { struct cs_spec *spec = codec->spec;
/* init_verb sequence for C0/C1/C2 errata*/ snd_hda_sequence_write(codec, cs_errata_init_verbs);
snd_hda_sequence_write(codec, cs_coef_init_verbs);
if (spec->gpio_mask) { snd_hda_codec_write(codec, 0x01, 0, AC_VERB_SET_GPIO_MASK, spec->gpio_mask);
Here, spec is NULL, so loading spec->gpio_mask causes a fatal pagefault at address 0x180 (ie the offset of gpio_mask in the struct). I was going to prepare a patch to guard for this, but since spec is expected in so many places, there is a likely a behavioural issue preventing a spec struct being generated.
Let me know for further testing and debug.
It's because of calling a leftover callback. The patch below should fix the issue. I'll queue it up to for-linus branch.
Oops, a wrong patch. The corrected one below.
Takashi
--- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] ALSA: hda - Fix Oops at codec reset/reconfig
snd_hda_codec_reset() calls restore_pincfgs() where the codec is powered up again, which eventually tries to resume and initialize via the callbacks of the codec. However, it's the place just after codec free callback, thus no codec callbacks should be called after that. On a codec like CS4206, it results in Oops due to the access in init callback.
This patch fixes the issue by clearing the codec callbacks properly after freeing codec.
Reported-by: Daniel J Blueman daniel@quora.org Cc: stable@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index f25c24c..1c65cc5 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2353,6 +2353,7 @@ int snd_hda_codec_reset(struct hda_codec *codec) } if (codec->patch_ops.free) codec->patch_ops.free(codec); + memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); snd_hda_jack_tbl_clear(codec); codec->proc_widget_hook = NULL; codec->spec = NULL; @@ -2368,7 +2369,6 @@ int snd_hda_codec_reset(struct hda_codec *codec) codec->num_pcms = 0; codec->pcm_info = NULL; codec->preset = NULL; - memset(&codec->patch_ops, 0, sizeof(codec->patch_ops)); codec->slave_dig_outs = NULL; codec->spdif_status_reset = 0; module_put(codec->owner);
participants (1)
-
Takashi Iwai