[FFmpeg-devel] [PATCH] opus_silk: fix out of array read in silk_lsf2lpc

[Andreas Cadhalpun]

On 14.12.2015 02:38, Michael Niedermayer wrote:
> On Sun, Dec 13, 2015 at 10:51:31PM +0100, Andreas Cadhalpun wrote:
>> nlsf can be negative, but a negative index for silk_cosine doesn't work.
>> ---
>>  libavcodec/opus_silk.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/opus_silk.c b/libavcodec/opus_silk.c
>> index 841d1ed..3ac83b8 100644
>> --- a/libavcodec/opus_silk.c
>> +++ b/libavcodec/opus_silk.c
>> @@ -941,7 +941,7 @@ static void silk_lsf2lpc(const int16_t nlsf[16], float lpcf[16], int order)
>>  
>>      /* convert the LSFs to LSPs, i.e. 2*cos(LSF) */
>>      for (k = 0; k < order; k++) {
>> -        int index = nlsf[k] >> 8;
>> +        int index = FFABS(nlsf[k]) >> 8;
>>          int offset = nlsf[k] & 255;
> 
> this looks a bit strange
> 
> if nlsf[] is allowed to be negative then i would have expected that
> both nlsf[] would use the absolute value or if its nt allowed to be
> negative then why is it ?

Actually the specification says it shouldn't be negative.
It's negative due to an overflow, when an intermediary uint16_t value
larger than INT16_MAX is assigned to nlsf.

> does the spec explain what should be done in this case ?

Not directly, but it implies that the overflow shouldn't happen [1]:
   Then, for each
   value of k from 0 to d_LPC-1, NLSF_Q15[k] is set to

             max(NLSF_Q15[k], NLSF_Q15[k-1] + NDeltaMin_Q15[k])

   Next, for each value of k from d_LPC-1 down to 0, NLSF_Q15[k] is set
   to

            min(NLSF_Q15[k], NLSF_Q15[k+1] - NDeltaMin_Q15[k+1])

> if not, its authors may want to amend it to clarify if this is
> invalid or undefined or something specific should be done

I think the specification is fine, just not our code.
Attached is a patch fixing this.

Best regards,
Andreas

1: https://tools.ietf.org/html/rfc6716
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-opus_silk-fix-int16_t-overflow-in-silk_stabilize_lsf.patch
Type: text/x-diff
Size: 1616 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151214/eec2095b/attachment.patch>