On Thu, 18 Feb 2016 07:16:29 +0100, Yang, Libin wrote:

Hi Takashi,

I submitted the patch which can fix a regression involved from my jack patch: commit 25e4abb33df3aafa7d1efba8f82f9178268efab1 Author: Libin Yang libin.yang@linux.intel.com Date: Tue Jan 12 11:13:27 2016 +0800

ALSA: hda - hdmi jack created based on pcm

I'm sorry for the inconvenience.

As the dyn pcm patches are merged into the for-next branch, I'm asking our QA to do the full test on it. They said they can start the stress test from next week.

One concern is the report from CI test about the crash at unloading the module. I couldn't reproduce it locally. Could you take a look?

Takashi

Regards, Libin

...

-----Original Message----- From: libin.yang@linux.intel.com [mailto:libin.yang@linux.intel.com] Sent: Thursday, February 18, 2016 1:25 PM To: alsa-devel@alsa-project.org; tiwai@suse.de Cc: Lin, Mengdong; Yang, Libin; Libin Yang Subject: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign

From: Libin Yang libin.yang@linux.intel.com

On Intel platform, if !dyn_pcm_assign, spec->pcm_rec[].jack is not NULL even after snd_hda_jack_tbl_clear() is called to free snd_jack. This may cause access invalid memory when calling snd_jack_report.

Please see more detail from: https://bugs.freedesktop.org/show_bug.cgi?id=94079

Signed-off-by: Libin Yang libin.yang@linux.intel.com

sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index f4443b5..3b47101 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct hda_codec *codec, { struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;

• struct hda_jack_tbl *jack_tbl; struct snd_jack *jack = NULL; int size;

@@ -1989,10 +1990,21 @@ static void sync_eld_via_acomp(struct hda_codec *codec, /* pcm_idx >=0 before update_eld() means it is in monitor * disconnected event. Jack must be fetched before update_eld() */

• if (per_pin->pcm_idx >= 0)

• /* if !dyn_pcm_assign, get jack from hda_jack_tbl

• * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not
  

• * NULL even after snd_hda_jack_tbl_clear() is called to
  

• * free snd_jack. This may cause access invalid memory
  

• * when calling snd_jack_report
  

• */
  

• if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) jack = spec->pcm_rec[per_pin->pcm_idx].jack;
• else {

• jack_tbl = snd_hda_jack_tbl_get(codec, per_pin-
  

...

pin_nid);

• if (jack_tbl)
  

• 	jack = jack_tbl->jack;
  

• } update_eld(codec, per_pin, eld);

• if (jack == NULL && per_pin->pcm_idx >= 0)

• if (jack == NULL && per_pin->pcm_idx >= 0 && spec-

...

dyn_pcm_assign)

jack = spec->pcm_rec[per_pin->pcm_idx].jack;

if (jack == NULL) goto unlock; -- 1.9.1

Hi Takashi,

-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, February 18, 2016 3:54 PM To: libin.yang@linux.intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign

On Thu, 18 Feb 2016 06:25:02 +0100, libin.yang@linux.intel.com wrote:

...

From: Libin Yang libin.yang@linux.intel.com

On Intel platform, if !dyn_pcm_assign, spec->pcm_rec[].jack is not NULL even after snd_hda_jack_tbl_clear() is called to free snd_jack. This may cause access invalid memory when calling snd_jack_report.

Please see more detail from: https://bugs.freedesktop.org/show_bug.cgi?id=94079

Signed-off-by: Libin Yang libin.yang@linux.intel.com

sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c

b/sound/pci/hda/patch_hdmi.c

...

index f4443b5..3b47101 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct

hda_codec *codec,

...

{ struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;

• struct hda_jack_tbl *jack_tbl; struct snd_jack *jack = NULL; int size;

@@ -1989,10 +1990,21 @@ static void sync_eld_via_acomp(struct

hda_codec *codec,

...

/* pcm_idx >=0 before update_eld() means it is in monitor * disconnected event. Jack must be fetched before update_eld() */

• if (per_pin->pcm_idx >= 0)

• /* if !dyn_pcm_assign, get jack from hda_jack_tbl

• * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not
  

• * NULL even after snd_hda_jack_tbl_clear() is called to
  

• * free snd_jack. This may cause access invalid memory
  

• * when calling snd_jack_report
  

• */
  

• if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) jack = spec->pcm_rec[per_pin->pcm_idx].jack;
• else {

• jack_tbl = snd_hda_jack_tbl_get(codec, per_pin-
  

pin_nid);

• if (jack_tbl)
  

• 	jack = jack_tbl->jack;
  

• } update_eld(codec, per_pin, eld);

• if (jack == NULL && per_pin->pcm_idx >= 0)

• if (jack == NULL && per_pin->pcm_idx >= 0 && spec-

dyn_pcm_assign) jack = spec->pcm_rec[per_pin->pcm_idx].jack; if (jack == NULL) goto unlock;

I'd create a separate small helper function, e.g. pin_to_jack() doing like:

pin_idx_to_jack(codec, per_pin) { if (spec->dyn_pcm_assign) { if (per_pin->pcm_idx < 0) return NULL; return spec->pcm_rec[per_pin->pcm_idx].jack; } else { jack_tbl = snd_hda_jack_tbl_get(codec, per_pin-

...

pin_nid);

if (!jack_tbl)
	return NULL;
return jack_tbl->jack;

} }

Then the code update_eld() will be cleaner,

jack = pin_to_jack(codec, per_pin); update_eld(codec, per_pin, eld); if (!jack) jack = pin_to_jack(codec, per_pin); if (!jack) goto unlock;

Alternatively, assign pcm_rec[].jack beforehand for non-dyn PCM. It's fixed, so you can assign it in initialization.

Get it. So your patch will be merged to for-next soon? Thanks.

Regards, Libin

thanks,

Takashi

Hi Takashi,

-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, February 18, 2016 4:07 PM To: Yang, Libin Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin, Mengdong Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign

On Thu, 18 Feb 2016 09:02:24 +0100, Yang, Libin wrote:

...

Hi Takashi,

...

-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, February 18, 2016 3:54 PM To: libin.yang@linux.intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl

when

...

...

not dyn_pcm_assign

On Thu, 18 Feb 2016 06:25:02 +0100, libin.yang@linux.intel.com wrote:

...

From: Libin Yang libin.yang@linux.intel.com

On Intel platform, if !dyn_pcm_assign, spec->pcm_rec[].jack is not NULL even after snd_hda_jack_tbl_clear() is called to free snd_jack. This may cause access invalid memory when calling snd_jack_report.

Please see more detail from: https://bugs.freedesktop.org/show_bug.cgi?id=94079

Signed-off-by: Libin Yang libin.yang@linux.intel.com

sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c

b/sound/pci/hda/patch_hdmi.c

...

index f4443b5..3b47101 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct

hda_codec *codec,

...

{ struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;

• struct hda_jack_tbl *jack_tbl; struct snd_jack *jack = NULL; int size;

@@ -1989,10 +1990,21 @@ static void

sync_eld_via_acomp(struct

...

...

hda_codec *codec,

...

/* pcm_idx >=0 before update_eld() means it is in monitor * disconnected event. Jack must be fetched before update_eld() */

• if (per_pin->pcm_idx >= 0)

• /* if !dyn_pcm_assign, get jack from hda_jack_tbl

• * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not
  

• * NULL even after snd_hda_jack_tbl_clear() is called to
  

• * free snd_jack. This may cause access invalid memory
  

• * when calling snd_jack_report
  

• */
  

• if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) jack = spec->pcm_rec[per_pin->pcm_idx].jack;
• else {

• jack_tbl = snd_hda_jack_tbl_get(codec, per_pin-
  

pin_nid);

• if (jack_tbl)
  

• 	jack = jack_tbl->jack;
  

• } update_eld(codec, per_pin, eld);

• if (jack == NULL && per_pin->pcm_idx >= 0)

• if (jack == NULL && per_pin->pcm_idx >= 0 && spec-

dyn_pcm_assign) jack = spec->pcm_rec[per_pin->pcm_idx].jack; if (jack == NULL) goto unlock;

I'd create a separate small helper function, e.g. pin_to_jack() doing like:

pin_idx_to_jack(codec, per_pin) { if (spec->dyn_pcm_assign) { if (per_pin->pcm_idx < 0) return NULL; return spec->pcm_rec[per_pin->pcm_idx].jack; } else { jack_tbl = snd_hda_jack_tbl_get(codec, per_pin-

...

pin_nid);

if (!jack_tbl)
	return NULL;
return jack_tbl->jack;

} }

Then the code update_eld() will be cleaner,

jack = pin_to_jack(codec, per_pin); update_eld(codec, per_pin, eld); if (!jack) jack = pin_to_jack(codec, per_pin); if (!jack) goto unlock;

Alternatively, assign pcm_rec[].jack beforehand for non-dyn PCM. It's fixed, so you can assign it in initialization.

Get it. So your patch will be merged to for-next soon? Thanks.

I *would* do, but I won't write it but wait for your renewed patch ;)

I misunderstand that you have already written such patch.

I will refine the patch. :)

Regards, Libin

Takashi