On Wed, Oct 28, 2015 at 03:16:34PM +0100, Hendrik Leppkes wrote:
> On Wed, Oct 28, 2015 at 2:35 PM, Luca Barbato <[email protected]> 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 <[email protected]> wrote:
> >>>> On Mon, Oct 26, 2015 at 11:29 PM, Kieran Kunhya <[email protected]> wrote:
> >>>>> From a1314d5c9774d555718bbc0a8612144c890bbc59 Mon Sep 17 00:00:00 2001
> >>>>> From: Kieran Kunhya <[email protected]>
> >>>>> 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 <[email protected]>
> >>> 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?
> Instead, you want to add error checks into the ASM? C does error
> checks, ASM does optimized algorithms, no checking.

I think Luca's point was that only the x86 implementation crashes, so that
implementation should be fixed instead of working around it in the C code.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to