[alsa-devel] [PATCH v4 2/5] ALSA: hda - hdmi: Do not expose eld data when eld is invalid

[Takashi Iwai]

At Tue, 19 Feb 2013 13:23:02 +0100,
David Henningsson wrote:
> 
> Previously, it was possible to read the eld data of the previous
> monitor connected. This should not be allowed.
> 
> Also refactor the function slightly.
> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  sound/pci/hda/patch_hdmi.c |   23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 32adaa6..7a73dfb 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -343,14 +343,16 @@ static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
>  			struct snd_ctl_elem_info *uinfo)
>  {
>  	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> -	struct hdmi_spec *spec;
> +	struct hdmi_spec *spec = codec->spec;
> +	struct hdmi_eld *eld;
>  	int pin_idx;
>  
> -	spec = codec->spec;
>  	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
>  
>  	pin_idx = kcontrol->private_value;
> -	uinfo->count = spec->pins[pin_idx].sink_eld.eld_size;
> +	eld = &spec->pins[pin_idx].sink_eld;
> +
> +	uinfo->count = eld->eld_valid ? eld->eld_size : 0;
>  
>  	return 0;
>  }
> @@ -359,14 +361,21 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
>  			struct snd_ctl_elem_value *ucontrol)
>  {
>  	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> -	struct hdmi_spec *spec;
> +	struct hdmi_spec *spec = codec->spec;
> +	struct hdmi_eld *eld;
>  	int pin_idx;
>  
> -	spec = codec->spec;
>  	pin_idx = kcontrol->private_value;
> +	eld = &spec->pins[pin_idx].sink_eld;
> +
> +	if (eld->eld_size > ARRAY_SIZE(ucontrol->value.bytes.data)) {
> +		snd_BUG();
> +		return -EINVAL;
> +	}
>  
> -	memcpy(ucontrol->value.bytes.data,
> -		spec->pins[pin_idx].sink_eld.eld_buffer, ELD_MAX_SIZE);
> +	if (eld->eld_valid)
> +		memcpy(ucontrol->value.bytes.data, eld->eld_buffer,
> +		       eld->eld_size);

Looking at the code again, I think it's safer to clear the array at
first.  In theory, it's possible that the ELD changes between info()
and get().  For example, when it's unplugged between info() and get()
calls, info() returns certain ELD bytes while get() leaves just
uninitialized.  If it's all zero, at least, it's easier to see that
something is wrong.


thanks,

Takashi