* [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
@ 2016-02-19 7:42 libin.yang
2016-02-19 8:20 ` Takashi Iwai
0 siblings, 1 reply; 13+ messages in thread
From: libin.yang @ 2016-02-19 7:42 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
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.
Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
sound/pci/hda/patch_hdmi.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index f4443b5..541986f 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1956,6 +1956,29 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
return ret;
}
+static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
+ struct hdmi_spec_per_pin *per_pin)
+{
+ struct hdmi_spec *spec = codec->spec;
+ struct snd_jack *jack = NULL;
+ struct hda_jack_tbl *jack_tbl;
+
+ /* 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 if (!spec->dyn_pcm_assign) {
+ jack_tbl = snd_hda_jack_tbl_get(codec, per_pin->pin_nid);
+ if (jack_tbl)
+ jack = jack_tbl->jack;
+ }
+ return jack;
+}
+
/* update ELD and jack state via audio component */
static void sync_eld_via_acomp(struct hda_codec *codec,
struct hdmi_spec_per_pin *per_pin)
@@ -1989,11 +2012,10 @@ 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)
- jack = spec->pcm_rec[per_pin->pcm_idx].jack;
+ jack = pin_idx_to_jack(codec, per_pin);
update_eld(codec, per_pin, eld);
- if (jack == NULL && per_pin->pcm_idx >= 0)
- jack = spec->pcm_rec[per_pin->pcm_idx].jack;
+ if (jack == NULL)
+ jack = pin_idx_to_jack(codec, per_pin);
if (jack == NULL)
goto unlock;
snd_jack_report(jack,
--
1.9.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-19 7:42 [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign libin.yang
@ 2016-02-19 8:20 ` Takashi Iwai
0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2016-02-19 8:20 UTC (permalink / raw)
To: libin.yang; +Cc: libin.yang, mengdong.lin, alsa-devel
On Fri, 19 Feb 2016 08:42:06 +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.
>
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
Applied, thanks.
Takashi
> ---
> sound/pci/hda/patch_hdmi.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index f4443b5..541986f 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1956,6 +1956,29 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
> return ret;
> }
>
> +static struct snd_jack *pin_idx_to_jack(struct hda_codec *codec,
> + struct hdmi_spec_per_pin *per_pin)
> +{
> + struct hdmi_spec *spec = codec->spec;
> + struct snd_jack *jack = NULL;
> + struct hda_jack_tbl *jack_tbl;
> +
> + /* 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 if (!spec->dyn_pcm_assign) {
> + jack_tbl = snd_hda_jack_tbl_get(codec, per_pin->pin_nid);
> + if (jack_tbl)
> + jack = jack_tbl->jack;
> + }
> + return jack;
> +}
> +
> /* update ELD and jack state via audio component */
> static void sync_eld_via_acomp(struct hda_codec *codec,
> struct hdmi_spec_per_pin *per_pin)
> @@ -1989,11 +2012,10 @@ 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)
> - jack = spec->pcm_rec[per_pin->pcm_idx].jack;
> + jack = pin_idx_to_jack(codec, per_pin);
> update_eld(codec, per_pin, eld);
> - if (jack == NULL && per_pin->pcm_idx >= 0)
> - jack = spec->pcm_rec[per_pin->pcm_idx].jack;
> + if (jack == NULL)
> + jack = pin_idx_to_jack(codec, per_pin);
> if (jack == NULL)
> goto unlock;
> snd_jack_report(jack,
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
@ 2016-02-18 5:25 libin.yang
2016-02-18 6:16 ` Yang, Libin
2016-02-18 7:53 ` Takashi Iwai
0 siblings, 2 replies; 13+ messages in thread
From: libin.yang @ 2016-02-18 5:25 UTC (permalink / raw)
To: alsa-devel, tiwai; +Cc: libin.yang, mengdong.lin, Libin Yang
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 5:25 libin.yang
@ 2016-02-18 6:16 ` Yang, Libin
2016-02-18 7:54 ` Takashi Iwai
2016-02-18 7:53 ` Takashi Iwai
1 sibling, 1 reply; 13+ messages in thread
From: Yang, Libin @ 2016-02-18 6:16 UTC (permalink / raw)
To: libin.yang@linux.intel.com, alsa-devel@alsa-project.org,
tiwai@suse.de
Cc: Lin, Mengdong
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.
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 6:16 ` Yang, Libin
@ 2016-02-18 7:54 ` Takashi Iwai
2016-02-18 8:06 ` Yang, Libin
0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-02-18 7:54 UTC (permalink / raw)
To: Yang, Libin
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 7:54 ` Takashi Iwai
@ 2016-02-18 8:06 ` Yang, Libin
2016-02-18 8:08 ` Takashi Iwai
0 siblings, 1 reply; 13+ messages in thread
From: Yang, Libin @ 2016-02-18 8:06 UTC (permalink / raw)
To: Takashi Iwai
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
Hi Takashi,
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, February 18, 2016 3:55 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 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?
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
>
>
> 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
> >
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 8:06 ` Yang, Libin
@ 2016-02-18 8:08 ` Takashi Iwai
2016-02-18 8:23 ` Yang, Libin
0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-02-18 8:08 UTC (permalink / raw)
To: Yang, Libin
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
On Thu, 18 Feb 2016 09:06:47 +0100,
Yang, Libin wrote:
>
> Hi Takashi,
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Thursday, February 18, 2016 3:55 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 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?
>
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 8:08 ` Takashi Iwai
@ 2016-02-18 8:23 ` Yang, Libin
2016-02-18 9:03 ` Takashi Iwai
0 siblings, 1 reply; 13+ messages in thread
From: Yang, Libin @ 2016-02-18 8:23 UTC (permalink / raw)
To: Takashi Iwai
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, February 18, 2016 4:09 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:06:47 +0100,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Thursday, February 18, 2016 3:55 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 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?
> >
> > 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?
>
>
> Takashi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 8:23 ` Yang, Libin
@ 2016-02-18 9:03 ` Takashi Iwai
0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2016-02-18 9:03 UTC (permalink / raw)
To: Yang, Libin
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
On Thu, 18 Feb 2016 09:23:43 +0100,
Yang, Libin wrote:
>
>
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Thursday, February 18, 2016 4:09 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:06:47 +0100,
> > Yang, Libin wrote:
> > >
> > > Hi Takashi,
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Thursday, February 18, 2016 3:55 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 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?
> > >
> > > 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?
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 5:25 libin.yang
2016-02-18 6:16 ` Yang, Libin
@ 2016-02-18 7:53 ` Takashi Iwai
2016-02-18 8:02 ` Yang, Libin
1 sibling, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-02-18 7:53 UTC (permalink / raw)
To: libin.yang; +Cc: libin.yang, mengdong.lin, alsa-devel
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.
thanks,
Takashi
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 7:53 ` Takashi Iwai
@ 2016-02-18 8:02 ` Yang, Libin
2016-02-18 8:07 ` Takashi Iwai
0 siblings, 1 reply; 13+ messages in thread
From: Yang, Libin @ 2016-02-18 8:02 UTC (permalink / raw)
To: Takashi Iwai, libin.yang@linux.intel.com
Cc: Lin, Mengdong, alsa-devel@alsa-project.org
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 8:02 ` Yang, Libin
@ 2016-02-18 8:07 ` Takashi Iwai
2016-02-18 8:11 ` Yang, Libin
0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-02-18 8:07 UTC (permalink / raw)
To: Yang, Libin
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
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 ;)
Takashi
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign
2016-02-18 8:07 ` Takashi Iwai
@ 2016-02-18 8:11 ` Yang, Libin
0 siblings, 0 replies; 13+ messages in thread
From: Yang, Libin @ 2016-02-18 8:11 UTC (permalink / raw)
To: Takashi Iwai
Cc: Lin, Mengdong, libin.yang@linux.intel.com,
alsa-devel@alsa-project.org
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-02-19 8:20 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-19 7:42 [PATCH] ALSA: hda - hdmi get jack from hda_jack_tbl when not dyn_pcm_assign libin.yang
2016-02-19 8:20 ` Takashi Iwai
-- strict thread matches above, loose matches on Subject: below --
2016-02-18 5:25 libin.yang
2016-02-18 6:16 ` Yang, Libin
2016-02-18 7:54 ` Takashi Iwai
2016-02-18 8:06 ` Yang, Libin
2016-02-18 8:08 ` Takashi Iwai
2016-02-18 8:23 ` Yang, Libin
2016-02-18 9:03 ` Takashi Iwai
2016-02-18 7:53 ` Takashi Iwai
2016-02-18 8:02 ` Yang, Libin
2016-02-18 8:07 ` Takashi Iwai
2016-02-18 8:11 ` Yang, Libin
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.