Re: [PATCH] amixer: add support for TLV byte control read

[Takashi Iwai <tiwai@suse.de>]

On Fri, 22 Jan 2016 06:46:52 +0100,
Vinod Koul wrote:
> 
> TLV byte control are new type of byte controls added in kernel where
> controls can have large sizes.
>
> For these controls querying with 4096 size fails, so use the queried size to
> read the control
> 
> This fixes the crash with current cget/contents on these type of controls

Hmm...  Theoretically the TLV size and the element size are
independent.  So, it's not good to check only the type being
SND_CTL_ELEM_TYPE_BYTES.  This can be used legally by other cases,
too.

Basically it is an abuse in ASoC side to return the size of
TLV by the element's info.  Usually such value would lead to crash,
but the unique point of ASoC ext bytes ctl is that it doesn't have RW
accesses but only TLV_RW accesses.

So, instead of only checking the type being BYTES, check the
accesses.  Only when all these conditions are met, we may take the
count as TLV (element) size.  (And still we should have the sanity
check of the value, too.)

Yet, this isn't a really "fix" for the crash.  Even without the patch
it shouldn't crash -- it should receive 4096 bytes, and tries to
decode.  Where did you get the crash exactly?


thanks,

Takashi

> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  amixer/amixer.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/amixer/amixer.c b/amixer/amixer.c
> index db1849333da3..cfe13592347f 100644
> --- a/amixer/amixer.c
> +++ b/amixer/amixer.c
> @@ -588,7 +588,7 @@ static int show_control(const char *space, snd_hctl_elem_t *elem,
>  			int level)
>  {
>  	int err;
> -	unsigned int item, idx, count, *tlv;
> +	unsigned int item, idx, count, *tlv, tlv_sz;
>  	snd_ctl_elem_type_t type;
>  	snd_ctl_elem_id_t *id;
>  	snd_ctl_elem_info_t *info;
> @@ -682,13 +682,20 @@ static int show_control(const char *space, snd_hctl_elem_t *elem,
>  	      __skip_read:
>  		if (!snd_ctl_elem_info_is_tlv_readable(info))
>  			goto __skip_tlv;
> -		tlv = malloc(4096);
> -		if ((err = snd_hctl_elem_tlv_read(elem, tlv, 4096)) < 0) {
> +
> +		if (type == SND_CTL_ELEM_TYPE_BYTES)
> +			tlv_sz = count + 2 * sizeof(unsigned int);
> +		else
> +			tlv_sz = 4096;
> +
> +		tlv = malloc(tlv_sz);
> +
> +		if ((err = snd_hctl_elem_tlv_read(elem, tlv, tlv_sz)) < 0) {
>  			error("Control %s element TLV read error: %s\n", card, snd_strerror(err));
>  			free(tlv);
>  			return err;
>  		}
> -		decode_tlv(strlen(space), tlv, 4096);
> +		decode_tlv(strlen(space), tlv, tlv_sz);
>  		free(tlv);
>  	}
>        __skip_tlv:
> -- 
> 1.9.1
>