[alsa-devel] [PATCH] amixer: add support for TLV byte control read

[Takashi Iwai]

On Thu, 28 Jan 2016 10:26:44 +0100,
Vinod Koul wrote:
> 
> On Thu, Jan 28, 2016 at 06:49:48AM +0100, Takashi Iwai wrote:
> > > Yes did, but didn't get a clue. tlv seems to be valid before free and points
> > > to right location and contents
> > 
> > Could you show the backtrace?
> 
> 676                     if ((err = snd_hctl_elem_tlv_read(elem, tlv, 4096)) < 0) {
> (gdb) n
> 677                             error("Control %s element TLV read error: %s\n", card, snd_strerror(err));
> (gdb) p err
> $1 = -14
> 
> (gdb) n
> amixer: Control hw:0 element TLV read error: Bad address
> 
> 678                             free(tlv);
> (gdb) p tlv
> $2 = (unsigned int *) 0x625f10
> (gdb) p tlv[0]
> $3 = 4294967295
> (gdb) p tlv[1]
> $4 = 0
> (gdb) backtrace
> #0  show_control (elem=0x625310, level=level at entry=5, space=0x409b01 "  ")
> at amixer.c:678
> #1  0x00000000004066d9 in cset (argc=argc at entry=1, argv=0x7fffffffe530,
> roflag=roflag at entry=1, keep_handle=keep_handle at entry=0) at amixer.c:1184
> #2  0x0000000000404352 in main (argc=<optimized out>, argv=0x7fffffffe518)
> at amixer.c:1863
> (gdb) c
> Continuing.

The line number doesn't match with the latest code in git, so
double-check that the problem happens with the latest alsa-lib and
alsa-utils, too.

I'm thinking whether this is rather an issue in the kernel driver
side.  In skl_tplg_tlv_control_get(),

	if (bc->params) {
		if (copy_to_user(data, &bc->param_id, sizeof(u32)))
			return -EFAULT;
		if (copy_to_user(data + 1, &size, sizeof(u32)))
			return -EFAULT;
		if (copy_to_user(data + 2, bc->params, size))
			return -EFAULT;
	}

But here, size is the size of the whole container, not the size in the
container.  In the code above, you're copying size+8 bytes total and
this breaks the boundary already.


Takashi