diff mbox series

[FFmpeg-devel,5/7] avcodec/sonic: Fix several integer state overflows

Message ID 20200510192019.14218-5-michael@niedermayer.cc
State Accepted
Commit 61d9bf514de0acf256aa554e0c431e7c91e42a5c
Headers show
Series [FFmpeg-devel,1/7] avcodec/mpc: Fix multiple numerical overflows in ff_mpc_dequantize_and_synth()
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate fail Make fate failed

Commit Message

Michael Niedermayer May 10, 2020, 7:20 p.m. UTC
Fixes: signed integer overflow: -234 * -14797801 cannot be represented in type 'int'
Fixes: 20492/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SONIC_fuzzer-5695924975435776

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/sonic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Paul B Mahol May 10, 2020, 11:07 p.m. UTC | #1
NAK

please remove this code fully.

On 5/10/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> Fixes: signed integer overflow: -234 * -14797801 cannot be represented in
> type 'int'
> Fixes:
> 20492/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SONIC_fuzzer-5695924975435776
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/sonic.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/sonic.c b/libavcodec/sonic.c
> index b82c44344c..ea6ef10c9e 100644
> --- a/libavcodec/sonic.c
> +++ b/libavcodec/sonic.c
> @@ -458,8 +458,8 @@ static void predictor_init_state(int *k, int *state, int
> order)
>
>          for (j = 0, p = i+1; p < order; j++,p++)
>              {
> -            int tmp = x + shift_down(k[j] * state[p], LATTICE_SHIFT);
> -            state[p] += shift_down(k[j]*x, LATTICE_SHIFT);
> +            int tmp = x + shift_down(k[j] * (unsigned)state[p],
> LATTICE_SHIFT);
> +            state[p] += shift_down(k[j]* (unsigned)x, LATTICE_SHIFT);
>              x = tmp;
>          }
>      }
> @@ -467,7 +467,7 @@ static void predictor_init_state(int *k, int *state, int
> order)
>
>  static int predictor_calc_error(int *k, int *state, int order, int error)
>  {
> -    int i, x = error - shift_down(k[order-1] * state[order-1],
> LATTICE_SHIFT);
> +    int i, x = error - shift_down(k[order-1] *  (unsigned)state[order-1],
> LATTICE_SHIFT);
>
>  #if 1
>      int *k_ptr = &(k[order-2]),
> --
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Michael Niedermayer May 20, 2020, 6:59 p.m. UTC | #2
On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
> NAK
> 
> please remove this code fully.

Id like to apply this and backport to the releases branches because
sonic is in the releases
are you ok with that ?


Thanks

[...]
Michael Niedermayer June 11, 2020, 12:58 p.m. UTC | #3
On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
> > NAK
> > 
> > please remove this code fully.
> 
> Id like to apply this and backport to the releases branches because
> sonic is in the releases
> are you ok with that ?

will apply soon unless i hear objections

[...]
Paul B Mahol June 11, 2020, 3:37 p.m. UTC | #4
On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
>> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
>> > NAK
>> >
>> > please remove this code fully.
>>
>> Id like to apply this and backport to the releases branches because
>> sonic is in the releases
>> are you ok with that ?
>
> will apply soon unless i hear objections
>

NO, I'm not ok with that. I told you what to do with this code already.

> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> In fact, the RIAA has been known to suggest that students drop out
> of college or go to community college in order to be able to afford
> settlements. -- The RIAA
>
Jean-Baptiste Kempf June 14, 2020, 3:35 p.m. UTC | #5
On Thu, Jun 11, 2020, at 17:37, Paul B Mahol wrote:
> On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
> >> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
> >> > NAK
> >> >
> >> > please remove this code fully.
> >>
> >> Id like to apply this and backport to the releases branches because
> >> sonic is in the releases
> >> are you ok with that ?
> >
> > will apply soon unless i hear objections
> >
> 
> NO, I'm not ok with that. I told you what to do with this code already.

Unfortunately, there is no consensus on this code.
So, before removing (or not), a fix is necessary.
Paul B Mahol June 14, 2020, 3:58 p.m. UTC | #6
On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
> On Thu, Jun 11, 2020, at 17:37, Paul B Mahol wrote:
>> On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> > On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
>> >> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
>> >> > NAK
>> >> >
>> >> > please remove this code fully.
>> >>
>> >> Id like to apply this and backport to the releases branches because
>> >> sonic is in the releases
>> >> are you ok with that ?
>> >
>> > will apply soon unless i hear objections
>> >
>>
>> NO, I'm not ok with that. I told you what to do with this code already.
>
> Unfortunately, there is no consensus on this code.
> So, before removing (or not), a fix is necessary.


You are last to talk about it, as you do not use this code.

Removing it will fix all issues with this long time abandoned code.

Instead you are Michael commit boost supporter.


>
> --
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jean-Baptiste Kempf June 14, 2020, 4:12 p.m. UTC | #7
On Sun, Jun 14, 2020, at 17:58, Paul B Mahol wrote:
> On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
> > On Thu, Jun 11, 2020, at 17:37, Paul B Mahol wrote:
> >> On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
> >> > On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
> >> >> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
> >> >> > NAK
> >> >> >
> >> >> > please remove this code fully.
> >> >>
> >> >> Id like to apply this and backport to the releases branches because
> >> >> sonic is in the releases
> >> >> are you ok with that ?
> >> >
> >> > will apply soon unless i hear objections
> >> >
> >>
> >> NO, I'm not ok with that. I told you what to do with this code already.
> >
> > Unfortunately, there is no consensus on this code.
> > So, before removing (or not), a fix is necessary.
> 
> You are last to talk about it, as you do not use this code.
>
> Instead you are Michael commit boost supporter.

I am not your friend. You have absolutely no right to talk to me like this.

Screaming your opinions louder does not make them the opinions of the community.
If you want this code remove, get it removed by either  consensus or a vote. Not by screaming.
Paul B Mahol June 14, 2020, 4:28 p.m. UTC | #8
On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
> On Sun, Jun 14, 2020, at 17:58, Paul B Mahol wrote:
>> On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
>> > On Thu, Jun 11, 2020, at 17:37, Paul B Mahol wrote:
>> >> On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> >> > On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
>> >> >> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
>> >> >> > NAK
>> >> >> >
>> >> >> > please remove this code fully.
>> >> >>
>> >> >> Id like to apply this and backport to the releases branches because
>> >> >> sonic is in the releases
>> >> >> are you ok with that ?
>> >> >
>> >> > will apply soon unless i hear objections
>> >> >
>> >>
>> >> NO, I'm not ok with that. I told you what to do with this code already.
>> >
>> > Unfortunately, there is no consensus on this code.
>> > So, before removing (or not), a fix is necessary.
>>
>> You are last to talk about it, as you do not use this code.
>>
>> Instead you are Michael commit boost supporter.
>
> I am not your friend. You have absolutely no right to talk to me like this.
>
> Screaming your opinions louder does not make them the opinions of the
> community.
> If you want this code remove, get it removed by either  consensus or a vote.
> Not by screaming.

You are mistaken, Mr. President.
This code is under codec cap experimental and can be removed at will.

>
> --
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jean-Baptiste Kempf June 14, 2020, 4:33 p.m. UTC | #9
On Sun, Jun 14, 2020, at 18:28, Paul B Mahol wrote:
> > If you want this code remove, get it removed by either  consensus or a vote.
> > Not by screaming.
> 
> You are mistaken, Mr. President.
> This code is under codec cap experimental and can be removed at will.

Experimental cap does not mean that the code can be removed at will.
It is a warning for users.

It does not mean I think the code should be kept (or not), just that we are a community, and decisions happen in a commune fashion.
James Almer June 14, 2020, 5:47 p.m. UTC | #10
On 6/14/2020 1:28 PM, Paul B Mahol wrote:
> On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
>> On Sun, Jun 14, 2020, at 17:58, Paul B Mahol wrote:
>>> On 6/14/20, Jean-Baptiste Kempf <jb@videolan.org> wrote:
>>>> On Thu, Jun 11, 2020, at 17:37, Paul B Mahol wrote:
>>>>> On 6/11/20, Michael Niedermayer <michael@niedermayer.cc> wrote:
>>>>>> On Wed, May 20, 2020 at 08:59:20PM +0200, Michael Niedermayer wrote:
>>>>>>> On Mon, May 11, 2020 at 01:07:54AM +0200, Paul B Mahol wrote:
>>>>>>>> NAK
>>>>>>>>
>>>>>>>> please remove this code fully.
>>>>>>>
>>>>>>> Id like to apply this and backport to the releases branches because
>>>>>>> sonic is in the releases
>>>>>>> are you ok with that ?
>>>>>>
>>>>>> will apply soon unless i hear objections
>>>>>>
>>>>>
>>>>> NO, I'm not ok with that. I told you what to do with this code already.
>>>>
>>>> Unfortunately, there is no consensus on this code.
>>>> So, before removing (or not), a fix is necessary.
>>>
>>> You are last to talk about it, as you do not use this code.
>>>
>>> Instead you are Michael commit boost supporter.
>>
>> I am not your friend. You have absolutely no right to talk to me like this.
>>
>> Screaming your opinions louder does not make them the opinions of the
>> community.
>> If you want this code remove, get it removed by either  consensus or a vote.
>> Not by screaming.
> 
> You are mistaken, Mr. President.
> This code is under codec cap experimental and can be removed at will.

From a technical PoV, it could be interpreted as allowed to be removed
without warning or deprecation period at any time. But it does not
override the requirement of having a project consensus about it.
diff mbox series

Patch

diff --git a/libavcodec/sonic.c b/libavcodec/sonic.c
index b82c44344c..ea6ef10c9e 100644
--- a/libavcodec/sonic.c
+++ b/libavcodec/sonic.c
@@ -458,8 +458,8 @@  static void predictor_init_state(int *k, int *state, int order)
 
         for (j = 0, p = i+1; p < order; j++,p++)
             {
-            int tmp = x + shift_down(k[j] * state[p], LATTICE_SHIFT);
-            state[p] += shift_down(k[j]*x, LATTICE_SHIFT);
+            int tmp = x + shift_down(k[j] * (unsigned)state[p], LATTICE_SHIFT);
+            state[p] += shift_down(k[j]* (unsigned)x, LATTICE_SHIFT);
             x = tmp;
         }
     }
@@ -467,7 +467,7 @@  static void predictor_init_state(int *k, int *state, int order)
 
 static int predictor_calc_error(int *k, int *state, int order, int error)
 {
-    int i, x = error - shift_down(k[order-1] * state[order-1], LATTICE_SHIFT);
+    int i, x = error - shift_down(k[order-1] *  (unsigned)state[order-1], LATTICE_SHIFT);
 
 #if 1
     int *k_ptr = &(k[order-2]),