Re: [PATCH 1/2] hda - fix ELD memory leak

[Wu Fengguang <fengguang.wu@intel.com>]

On Tue, Nov 15, 2011 at 10:35:41PM +0800, Takashi Iwai wrote:
> At Tue, 15 Nov 2011 22:31:55 +0800,
> Wu Fengguang wrote:
> > 
> > memset(eld) clears eld->proc_entry which will leak the struct
> > snd_info_entry when unloading the module.
> > 
> > Fix it by
> > - remove memset(eld)
> > - set eld->eld_valid to true _after_ all eld fields have been filled
> > - don't access the other eld fields when (eld->eld_valid == false)
> > 
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> 
> This should be send to stable kernel, too, right?
> It appears in 3.1, at least.

Yeah. Good point!  I'll resend it when everything goes fine.

Thanks,
Fengguang

> > ---
> >  sound/pci/hda/hda_eld.c    |    5 +----
> >  sound/pci/hda/patch_hdmi.c |   11 +++++------
> >  2 files changed, 6 insertions(+), 10 deletions(-)
> > 
> > --- linux.orig/sound/pci/hda/hda_eld.c	2011-11-15 21:02:25.000000000 +0800
> > +++ linux/sound/pci/hda/hda_eld.c	2011-11-15 21:13:46.000000000 +0800
> > @@ -297,10 +297,10 @@ static int hdmi_update_eld(struct hdmi_e
> >  					buf + ELD_FIXED_BYTES + mnl + 3 * i);
> >  	}
> >  
> > +	e->eld_valid = true;
> >  	return 0;
> >  
> >  out_fail:
> > -	e->eld_ver = 0;
> >  	return -EINVAL;
> >  }
> >  
> > @@ -323,9 +323,6 @@ int snd_hdmi_get_eld(struct hdmi_eld *el
> >  	 * ELD is valid, actual eld_size is assigned in hdmi_update_eld()
> >  	 */
> >  
> > -	if (!eld->eld_valid)
> > -		return -ENOENT;
> > -
> >  	size = snd_hdmi_get_eld_size(codec, nid);
> >  	if (size == 0) {
> >  		/* wfg: workaround for ASUS P5E-VM HDMI board */
> > --- linux.orig/sound/pci/hda/patch_hdmi.c	2011-11-15 21:02:25.000000000 +0800
> > +++ linux/sound/pci/hda/patch_hdmi.c	2011-11-15 21:13:42.000000000 +0800
> > @@ -980,20 +980,19 @@ static void hdmi_present_sense(struct hd
> >  	 * the unsolicited response to avoid custom WARs.
> >  	 */
> >  	int present = snd_hda_pin_sense(codec, pin_nid);
> > +	bool eld_valid = false;
> >  
> > -	memset(eld, 0, sizeof(*eld));
> > +	eld->eld_valid = false;
> >  
> >  	eld->monitor_present	= !!(present & AC_PINSENSE_PRESENCE);
> >  	if (eld->monitor_present)
> > -		eld->eld_valid	= !!(present & AC_PINSENSE_ELDV);
> > -	else
> > -		eld->eld_valid	= 0;
> > +		eld_valid	= !!(present & AC_PINSENSE_ELDV);
> >  
> >  	printk(KERN_INFO
> >  		"HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
> > -		codec->addr, pin_nid, eld->monitor_present, eld->eld_valid);
> > +		codec->addr, pin_nid, eld->monitor_present, eld_valid);
> >  
> > -	if (eld->eld_valid)
> > +	if (eld_valid)
> >  		if (!snd_hdmi_get_eld(eld, codec, pin_nid))
> >  			snd_hdmi_show_eld(eld);
> >  
> >