On Wed, Oct 28, 2015 at 3:50 PM, Luca Barbato <[email protected]> wrote: > On 28/10/15 15:16, 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? > > 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. >
I can only repeat the point I made before - ASM is not about safety, its about speed. If you can re-write it to not be a single cycle slower and not have this particular behavior, great. In the meantime, can we apply this patch and get the crash done with? FWIW, all other callers of vector_fmul_scalar don't seem to have a chance of the value being zero. - Hendrik _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
