On 28/10/15 15:16, Hendrik Leppkes wrote:
> On Wed, Oct 28, 2015 at 2:35 PM, Luca Barbato <lu_z...@gentoo.org> wrote:
>> On 27/10/15 01:30, Luca Barbato wrote:
>>> On 27/10/15 00:09, Kieran Kunhya wrote:
>>>> On 26 October 2015 at 22:48, Hendrik Leppkes <h.lepp...@gmail.com> wrote:
>>>>> On Mon, Oct 26, 2015 at 11:29 PM, Kieran Kunhya <kier...@obe.tv> wrote:
>>>>>> From a1314d5c9774d555718bbc0a8612144c890bbc59 Mon Sep 17 00:00:00 2001
>>>>>> From: Kieran Kunhya <kier...@obe.tv>
>>>>>> Date: Mon, 26 Oct 2015 22:26:35 +0000
>>>>>> Subject: [PATCH] opusdec: Don't run vector_fmul_scalar on zero length
>>>>>> arrays
>>>>>>
>>>>>> Fixes crashes on fuzzed files
>>>>>>
>>>>>> ---
>>>>>> libavcodec/opusdec.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c
>>>>>> index acae6e1..03dd872 100644
>>>>>> --- a/libavcodec/opusdec.c
>>>>>> +++ b/libavcodec/opusdec.c
>>>>>> @@ -587,7 +587,7 @@ static int opus_decode_packet(AVCodecContext
>>>>>> *avctx, void *data,
>>>>>> memset(frame->extended_data[i], 0, frame->linesize[0]);
>>>>>> }
>>>>>>
>>>>>> - if (c->gain_i) {
>>>>>> + if (c->gain_i && decoded_samples >= 8) {
>>>>>> c->fdsp.vector_fmul_scalar((float*)frame->extended_data[i],
>>>>>> (float*)frame->extended_data[i],
>>>>>> c->gain,
>>>>>> FFALIGN(decoded_samples, 8));
>>>>>
>>>>>> 0 might be less arbitrary.
>>>>
>>>> New version:
>>>>
>>>> From 25edf86e35773d419b4bcc7aeeb7b96d0f1ef958 Mon Sep 17 00:00:00 2001
>>>> From: Kieran Kunhya <kier...@obe.tv>
>>>> Date: Mon, 26 Oct 2015 23:08:31 +0000
>>>> Subject: [PATCH] opusdec: Don't run vector_fmul_scalar on zero length
>>>> arrays
>>>>
>>>> Fixes crashes on fuzzed files
>>>> ---
>>>> libavcodec/opusdec.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c
>>>> index acae6e1..93c72c3 100644
>>>> --- a/libavcodec/opusdec.c
>>>> +++ b/libavcodec/opusdec.c
>>>> @@ -587,7 +587,7 @@ static int opus_decode_packet(AVCodecContext
>>>> *avctx, void *data,
>>>> memset(frame->extended_data[i], 0, frame->linesize[0]);
>>>> }
>>>>
>>>> - if (c->gain_i) {
>>>> + if (c->gain_i && decoded_samples > 0) {
>>>> c->fdsp.vector_fmul_scalar((float*)frame->extended_data[i],
>>>> (float*)frame->extended_data[i],
>>>> c->gain, FFALIGN(decoded_samples,
>>>> 8));
>>>
>>> Which is the range of valid values here?
>>>
>>
>> The documentation states "multiple of 4", all the other implementation
>> of that function behave, Kostya suggests to fix the faulty
>> implementation and I'm not really fond to triplecheck that the other
>> uses and the future use of this function would have the same issue...
>
> I don't get this point at all. You already have to check any future
> use if its a multiple of 4, whats the harm in just checking that its
> not 0 in the same thought?
That 0 is a multiple of 4 so at least the documented constraint should
be updated.
> Instead, you want to add error checks into the ASM? C does error
> checks, ASM does optimized algorithms, no checking.
No, I want to make sure that the functions to behave as documented, it
can works either by making sure that the ASM implementation doesn't do
anything unexpected or making sure that the documentation and the usage
fit the new constraints.
We mandate to have proper padding to let those functions run once IIRC.
Either solution is fine as long we do not have dangerous leftovers,
making sure that the x86 assembly behaves is a single change so might be
safer.
lu
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
