ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign

PATCH In review [email protected] 3 participants 11 messages Feb 18, 2016 Sound

lore

Share & tools

lore

[email protected]

author

2016-02-18 05:29 UTC #0 lore raw

PATCH 1/1 ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign

From: Libin Yang <[email protected]>

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 <[email protected]>

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

sound/pci/hda/patch_hdmi.c +14 -2

index f4443b5..3b47101 100644
@@ -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;

Signature · 1 line

1.9.1

Yang, Libin

2016-02-18 06:16 UTC #1 lore raw

Hi Takashi,

I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1

Author: Libin Yang <[email protected]>
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.

Regards,
Libin

Quoted 64 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, February 18, 2016 1:25 PM
To: [email protected]; [email protected]
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 <[email protected]>
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 <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
sound/pci/hda/patch_hdmi.c +14 -2
index f4443b5..3b47101 100644
@@ -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)
sound/pci/hda/patch_hdmi.c +0 -1
 		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
 	if (jack == NULL)
 		goto unlock;
--
1.9.1

Takashi Iwai

2016-02-18 07:54 UTC #2 lore raw

Quoted 9 lines · Libin

Hi Takashi,
I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1
Author: Libin Yang <[email protected]>
Date:   Tue Jan 12 11:13:27 2016 +0800

Quoted 7 lines

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

Quoted 70 lines

Regards,
Libin
Quoted 64 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, February 18, 2016 1:25 PM
To: [email protected]; [email protected]
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 <[email protected]>
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 <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
sound/pci/hda/patch_hdmi.c +14 -2
index f4443b5..3b47101 100644
@@ -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)
sound/pci/hda/patch_hdmi.c +0 -1
 		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
 	if (jack == NULL)
 		goto unlock;
--
1.9.1

Yang, Libin

2016-02-18 08:06 UTC #3 lore raw

Hi Takashi,

Quoted 22 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:55 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 11 lines · Libin
Hi Takashi,
I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1
Author: Libin Yang <[email protected]>
Date:   Tue Jan 12 11:13:27 2016 +0800
ALSA: hda - hdmi jack created based on pcm

Quoted 8 lines

Quoted 5 lines
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?

There is the same oops when unloading snd modules on my side (it happens occasionally).
With the patch, the oops is fixed. But I didn't meet the crash. The system is still alive.

I'm asking for their test case. As soon as I get the test case, I will do the test.

Regards,
Libin

Quoted 76 lines

Takashi
Quoted 11 lines
Regards,
Libin
Quoted 6 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]]
Sent: Thursday, February 18, 2016 1:25 PM
To: [email protected]; [email protected]
Cc: Lin, Mengdong; Yang, Libin; Libin Yang
Subject: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when
not
Quoted 11 lines
Quoted 11 lines
dyn_pcm_assign
From: Libin Yang <[email protected]>
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
Quoted 6 lines
Review
Quoted 6 lines
Signed-off-by Libin Yang <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/sound/pci/hda/patch_hdmi.c
Quoted 42 lines
Quoted 41 lines
index f4443b5..3b47101 100644
sound/pci/hda/patch_hdmi.c +14 -2
@@ -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)
sound/pci/hda/patch_hdmi.c +0 -1
 		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
 	if (jack == NULL)
 		goto unlock;
--
1.9.1

Takashi Iwai

2016-02-18 08:08 UTC #4 lore raw

Quoted 31 lines · Libin

Hi Takashi,
Quoted 28 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:55 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 16 lines · Libin
Hi Takashi,
I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1
Author: Libin Yang <[email protected]>
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.

Quoted 7 lines

Quoted 2 lines
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?
There is the same oops when unloading snd modules on my side (it happens occasionally).
With the patch, the oops is fixed. But I didn't meet the crash. The system is still alive.

Hmm. I wonder it because the report was done for HSW, so your patch (I suppose you meaning the patch 'ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign') shouldn't change anything.

Takashi

Yang, Libin

2016-02-18 08:23 UTC #5 lore raw

Quoted 49 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 4:09 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 10 lines · Libin
Hi Takashi,
Quoted 7 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:55 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl
when
Quoted 13 lines
Quoted 13 lines
not dyn_pcm_assign
Quoted 9 lines · Libin
Hi Takashi,
I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1
Author: Libin Yang <[email protected]>
Date:   Tue Jan 12 11:13:27 2016 +0800
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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
Quoted 1 line
Quoted 1 line
Quoted 1 line
our QA to do the full test on it. They said they can start the stress
test
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
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?

Quoted 8 lines

Quoted 4 lines
There is the same oops when unloading snd modules on my side (it happens occasionally).
With the patch, the oops is fixed. But I didn't meet the crash. The system is still alive.
Hmm. I wonder it because the report was done for HSW, so your patch (I suppose you meaning the patch 'ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign') shouldn't change anything.

If so, my patch may not help.

BTW: could you tell me how you know that the report was done from the dmesg?

Quoted 3 lines

Takashi

Takashi Iwai

2016-02-18 09:03 UTC #6 lore raw

Quoted 55 lines · Libin

Quoted 55 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 4:09 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 10 lines · Libin
Hi Takashi,
Quoted 7 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:55 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl
when
Quoted 13 lines
Quoted 13 lines
not dyn_pcm_assign
Quoted 9 lines · Libin
Hi Takashi,
I submitted the patch which can fix a regression involved from my jack patch:
commit 25e4abb33df3aafa7d1efba8f82f9178268efab1
Author: Libin Yang <[email protected]>
Date:   Tue Jan 12 11:13:27 2016 +0800
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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
Quoted 1 line
Quoted 1 line
Quoted 1 line
our QA to do the full test on it. They said they can start the stress
test
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
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?
There is the same oops when unloading snd modules on my side (it happens occasionally).
With the patch, the oops is fixed. But I didn't meet the crash. The system is still alive.

Quoted 8 lines

Quoted 3 lines
Hmm. I wonder it because the report was done for HSW, so your patch (I suppose you meaning the patch 'ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign') shouldn't change anything.
If so, my patch may not help.
BTW: could you tell me how you know that the report was done from the dmesg?

Gabriel posted lspci output, too:

$lspci -nn
00:03.0 Audio device [0403]: Intel Corporation Xeon E3-1200 v3/4th Gen Core Processor HD Audio Controller [8086:0c0c] (rev 06) 00:1b.0 Audio device [0403]: Intel Corporation 8 Series/C220 Series Chipset High Definition Audio Controller [8086:8c20] (rev 05)

Takashi

Takashi Iwai

2016-02-18 07:53 UTC #7 lore raw

In reply to [email protected]

Quoted 41 lines · On Thu, 18 Feb 2016 06:25:02 +0100, [email protected] wrote:

From: Libin Yang <[email protected]>

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

Signed-off-by Libin Yang <[email protected]>

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

sound/pci/hda/patch_hdmi.c +9 -1

index f4443b5..3b47101 100644
@@ -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 {

sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct hda_codec *codec, · line 2010

Quoted 10 lines

sound/pci/hda/patch_hdmi.c +5 -1

+		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.

thanks,

Takashi

Yang, Libin

2016-02-18 08:02 UTC #8 lore raw

Hi Takashi,

Quoted 94 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:54 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 10 lines · On Thu, 18 Feb 2016 06:25:02 +0100, [email protected] wrote:
From: Libin Yang <[email protected]>
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
Review
Quoted 6 lines
Signed-off-by Libin Yang <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/sound/pci/hda/patch_hdmi.c
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct
Quoted 4 lines
index f4443b5..3b47101 100644
sound/pci/hda/patch_hdmi.c +0 -0
@@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct
hda_codec *codec,
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct
Quoted 8 lines
sound/pci/hda/patch_hdmi.c +1 -0
 {
 	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,
Quoted 23 lines
sound/pci/hda/patch_hdmi.c +13 -2
 	/* 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;
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct hda_codec *codec, · line 2010
Quoted 2 lines
sound/pci/hda/patch_hdmi.c +0 -0
 	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-
Quoted 1 line
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;

Quoted 2 lines

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

Quoted 5 lines

thanks,
Takashi

Takashi Iwai

2016-02-18 08:07 UTC #9 lore raw

Quoted 97 lines · Libin

Hi Takashi,
Quoted 94 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:54 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 10 lines · On Thu, 18 Feb 2016 06:25:02 +0100, [email protected] wrote:
From: Libin Yang <[email protected]>
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
Review
Quoted 6 lines
Signed-off-by Libin Yang <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/sound/pci/hda/patch_hdmi.c
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct
Quoted 4 lines
index f4443b5..3b47101 100644
sound/pci/hda/patch_hdmi.c +0 -0
@@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct
hda_codec *codec,
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct
Quoted 8 lines
sound/pci/hda/patch_hdmi.c +1 -0
 {
 	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,
Quoted 23 lines
sound/pci/hda/patch_hdmi.c +13 -2
 	/* 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;
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct hda_codec *codec, · line 2010
Quoted 2 lines
sound/pci/hda/patch_hdmi.c +0 -0
 	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-
Quoted 1 line
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;

Quoted 4 lines

Quoted 2 lines
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 ;)

Takashi

Yang, Libin

2016-02-18 08:12 UTC #10 lore raw

Hi Takashi,

Quoted 110 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 4:07 PM
To: Yang, Libin
Cc: [email protected]; [email protected]; Lin, Mengdong
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
Quoted 9 lines · Libin
Hi Takashi,
Quoted 6 lines
-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, February 18, 2016 3:54 PM
To: [email protected]
Cc: [email protected]; Lin, Mengdong; Yang, Libin
Subject: Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl
when
Quoted 26 lines
Quoted 26 lines
not dyn_pcm_assign
Quoted 10 lines · On Thu, 18 Feb 2016 06:25:02 +0100, [email protected] wrote:
From: Libin Yang <[email protected]>
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
Review
Quoted 6 lines
Signed-off-by Libin Yang <[email protected]>
---
 sound/pci/hda/patch_hdmi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
(quoted diff) +0 -0
b/sound/pci/hda/patch_hdmi.c
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct
Quoted 4 lines
index f4443b5..3b47101 100644
sound/pci/hda/patch_hdmi.c +0 -0
@@ -1962,6 +1962,7 @@ static void sync_eld_via_acomp(struct
hda_codec *codec,
Quoted 8 lines
Quoted 8 lines
sound/pci/hda/patch_hdmi.c static void
Quoted 8 lines
sound/pci/hda/patch_hdmi.c +1 -0
 {
 	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
Quoted 54 lines
Quoted 54 lines
hda_codec *codec,
Quoted 23 lines
sound/pci/hda/patch_hdmi.c +13 -2
 	/* 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;
sound/pci/hda/patch_hdmi.c static void sync_eld_via_acomp(struct hda_codec *codec, · line 2010
Quoted 2 lines
sound/pci/hda/patch_hdmi.c +0 -0
 	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-
Quoted 1 line
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;

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
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

Quoted 3 lines

Takashi