[alsa-devel] [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Takashi Iwai]

On Tue, 05 Apr 2016 10:53:24 +0200,
Mengdong Lin wrote:
> 
> 
> On 04/05/2016 02:14 PM, Takashi Iwai wrote:
> > On Tue, 05 Apr 2016 07:47:08 +0200,
> > Mengdong Lin wrote:
> >>
> >>
> >>
> >> On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> >>> On Wed, 30 Mar 2016 09:11:17 +0200,
> >>> mengdong.lin at linux.intel.com wrote:
> >>>>
> >>>> +		switch (type) {
> >>>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> >>>> +			len = strlen(value);
> >>>> +			if (len > 16 || len == 0) {
> >>>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
> >>>> +				goto err;
> >>>> +			}
> >>>> +
> >>>> +			memcpy(tuple->uuid, value, 16);
> >>>
> >>> This may still overflow :)
> >>> How about simply using elem_copy_text()?
> >>
> >> Sorry for the late reply.
> >>
> >> Would you mind me using uuid_parse() here?
> >> It can convert an input UUID string into the binary representation.
> >>
> >> An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user
> >> friendly for the text conf file. But this will add dependency on libuuid.
> >
> > Additional dependency is no-go, especially when the required change is
> > so trivial.  It's just a string copy, after all.
> >
> 
> Maybe we can just use strncpy(dest, src, 16), assuming the strncpy will 
> not try to write a "\0" at dest[16] that may cause overflow?

You seem to think of things more complicated than needed.

Just reread your code.  What if a shorter string value is passed there
to memcpy() call?  That's what I suggested as an overflow.


Takashi