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

[Takashi Iwai]

On Wed, 27 Jan 2016 18:47:52 +0100,
Vinod Koul wrote:
> 
> On Fri, Jan 22, 2016 at 11:50:08AM +0100, Takashi Iwai wrote:
> > On Fri, 22 Jan 2016 10:56:48 +0100,
> > Vinod Koul wrote:
> > > > Hm, it has a check
> > > > 
> > > > static int snd_ctl_hw_elem_tlv(snd_ctl_t *handle, int op_flag,
> > > > 			       unsigned int numid,
> > > > 			       unsigned int *tlv, unsigned int tlv_size)
> > > > {
> > > > 	....
> > > > 		if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) {
> > > > 			free(xtlv);
> > > > 			return -EFAULT;
> > > > 		}
> > > > 		memcpy(tlv, xtlv->tlv, xtlv->tlv[1] + 2 * sizeof(unsigned int));
> > > > 
> > > > Do you mean somewhere here triggers a crash?
> > > 
> > > Yes while it tried to memcpy, in my case 30K sized data to 4K buffer, so I
> > > allocated right one and got all the data
> > 
> > But it has the size check before memcpy()?
> 
> Okay I had some issues (yet again) so I rebuilt the ubuntu packages and tools and reran
> this.
> 
> I get:
> 
> $ amixer -c0 cget numid=16
> numid=16,iface=MIXER,name='mdl params'
>   ; type=BYTES,access=-----RW-,values=30336
> amixer: Control hw:0 element TLV read error: Bad address
> 
> Segmentation fault (core dumped)
> 
> And the alsa-lib returns an err and we try to free up tlv on error which
> should be fine here but fails
> 
> Why should free cause a failure here (on 1.0.27)
> 
> On latest lib from git. I get same failure but in alsa-lib when it tries to
> free xtlv on failure of this check
> 
> if (xtlv->tlv[1] + 2 * sizeof(unsigned int) > tlv_size) {
>                    free(xtlv);
> 
> Why would that happen...? What am I missing :(

No idea.  You should check it via gdb.

> Also on second thought, even after working around this by setting right size
> so that we don't hit failures (this patch), I get my screen spammed by 30K byes so based
> on TLV access and bytes type I am thinking to not read and display on cget.
> 
> We should add new options for these controls ..

As already mentioned, we may handle differently for ctl elements that
have no r/w access.  At easiest, just skip tlv read for such an
element.


Takashi