diff mbox series

[FFmpeg-devel] lavc/flacdec: Increase residual limit from INT_MAX to UINT_MAX

Message ID 20220405133747.318846-1-mvanb1@gmail.com
State Accepted
Commit 296d3bbe167f06168ac54724626006b43002833f
Headers show
Series [FFmpeg-devel] lavc/flacdec: Increase residual limit from INT_MAX to UINT_MAX | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_fate_aarch64_jetson success Make fate finished
andriy/make_aarch64_jetson warning New warnings during build
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Martijn van Beurden April 5, 2022, 1:37 p.m. UTC
---
 libavcodec/flacdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andreas Rheinhardt April 5, 2022, 4:04 p.m. UTC | #1
Martijn van Beurden:
> ---
>  libavcodec/flacdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
> index dd6026f9de..cb32d7cae8 100644
> --- a/libavcodec/flacdec.c
> +++ b/libavcodec/flacdec.c
> @@ -260,7 +260,7 @@ static int decode_residuals(FLACContext *s, int32_t *decoded, int pred_order)
>              for (; i < samples; i++)
>                  *decoded++ = get_sbits_long(&gb, tmp);
>          } else {
> -            int real_limit = tmp ? (INT_MAX >> tmp) + 2 : INT_MAX;
> +            int real_limit = (tmp > 1) ? (INT_MAX >> (tmp - 1)) + 2 : INT_MAX;
>              for (; i < samples; i++) {
>                  int v = get_sr_golomb_flac(&gb, tmp, real_limit, 1);
>                  if (v == 0x80000000){

Wouldn't it be possible to use unsigned for real_limit?

- Andreas
Martijn van Beurden April 5, 2022, 4:26 p.m. UTC | #2
Op di 5 apr. 2022 18:04 schreef Andreas Rheinhardt <
andreas.rheinhardt@outlook.com>:

>
> Wouldn't it be possible to use unsigned for real_limit?
>

The limit is passed to get_ur_golomb_jpegls which takes int. In that
function, the limit is subtracted from, and the function is used by quite a
few other codecs, so changing it seems risky.

Besides creating a cleaner flacdec.c, it wouldn't achieve much either. For
tmp = 0 or tmp = 1, reaching this limit means the file contains a INT_MAX
or INT_MAX/2 unary coded, so a single symbol of either 2GiB or 1GiB in size.

>
Martijn van Beurden April 6, 2022, 7:12 a.m. UTC | #3
Op di 5 apr. 2022 om 15:37 schreef Martijn van Beurden <mvanb1@gmail.com>:
>
> ---
>  libavcodec/flacdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
> index dd6026f9de..cb32d7cae8 100644
> --- a/libavcodec/flacdec.c
> +++ b/libavcodec/flacdec.c
> @@ -260,7 +260,7 @@ static int decode_residuals(FLACContext *s, int32_t *decoded, int pred_order)
>              for (; i < samples; i++)
>                  *decoded++ = get_sbits_long(&gb, tmp);
>          } else {
> -            int real_limit = tmp ? (INT_MAX >> tmp) + 2 : INT_MAX;
> +            int real_limit = (tmp > 1) ? (INT_MAX >> (tmp - 1)) + 2 : INT_MAX;
>              for (; i < samples; i++) {
>                  int v = get_sr_golomb_flac(&gb, tmp, real_limit, 1);
>                  if (v == 0x80000000){
> --
> 2.30.2
>

A file needing this patch to decode properly can be found here:
https://github.com/ktmf01/flac-test-files/blob/main/subset/63%20-%20predictor%20overflow%20check%2C%2024-bit.flac

Kind regards, Martijn van Beurden
Martijn van Beurden April 29, 2022, 2:48 p.m. UTC | #4
Op wo 6 apr. 2022 om 09:12 schreef Martijn van Beurden <mvanb1@gmail.com>:
>
> Op di 5 apr. 2022 om 15:37 schreef Martijn van Beurden <mvanb1@gmail.com>:
> >
> > ---
> >  libavcodec/flacdec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
> > index dd6026f9de..cb32d7cae8 100644
> > --- a/libavcodec/flacdec.c
> > +++ b/libavcodec/flacdec.c
> > @@ -260,7 +260,7 @@ static int decode_residuals(FLACContext *s, int32_t *decoded, int pred_order)
> >              for (; i < samples; i++)
> >                  *decoded++ = get_sbits_long(&gb, tmp);
> >          } else {
> > -            int real_limit = tmp ? (INT_MAX >> tmp) + 2 : INT_MAX;
> > +            int real_limit = (tmp > 1) ? (INT_MAX >> (tmp - 1)) + 2 : INT_MAX;
> >              for (; i < samples; i++) {
> >                  int v = get_sr_golomb_flac(&gb, tmp, real_limit, 1);
> >                  if (v == 0x80000000){
> > --
> > 2.30.2
> >
>
> A file needing this patch to decode properly can be found here:
> https://github.com/ktmf01/flac-test-files/blob/main/subset/63%20-%20predictor%20overflow%20check%2C%2024-bit.flac
>
> Kind regards, Martijn van Beurden

Hereby I'd like to once more bring this patch to the attention of the
mailinglist.
Michael Niedermayer April 30, 2022, 7:26 p.m. UTC | #5
On Fri, Apr 29, 2022 at 04:48:23PM +0200, Martijn van Beurden wrote:
> Op wo 6 apr. 2022 om 09:12 schreef Martijn van Beurden <mvanb1@gmail.com>:
> >
> > Op di 5 apr. 2022 om 15:37 schreef Martijn van Beurden <mvanb1@gmail.com>:
> > >
> > > ---
> > >  libavcodec/flacdec.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
> > > index dd6026f9de..cb32d7cae8 100644
> > > --- a/libavcodec/flacdec.c
> > > +++ b/libavcodec/flacdec.c
> > > @@ -260,7 +260,7 @@ static int decode_residuals(FLACContext *s, int32_t *decoded, int pred_order)
> > >              for (; i < samples; i++)
> > >                  *decoded++ = get_sbits_long(&gb, tmp);
> > >          } else {
> > > -            int real_limit = tmp ? (INT_MAX >> tmp) + 2 : INT_MAX;
> > > +            int real_limit = (tmp > 1) ? (INT_MAX >> (tmp - 1)) + 2 : INT_MAX;
> > >              for (; i < samples; i++) {
> > >                  int v = get_sr_golomb_flac(&gb, tmp, real_limit, 1);
> > >                  if (v == 0x80000000){
> > > --
> > > 2.30.2
> > >
> >
> > A file needing this patch to decode properly can be found here:
> > https://github.com/ktmf01/flac-test-files/blob/main/subset/63%20-%20predictor%20overflow%20check%2C%2024-bit.flac
> >
> > Kind regards, Martijn van Beurden
> 
> Hereby I'd like to once more bring this patch to the attention of the
> mailinglist.

will apply

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
index dd6026f9de..cb32d7cae8 100644
--- a/libavcodec/flacdec.c
+++ b/libavcodec/flacdec.c
@@ -260,7 +260,7 @@  static int decode_residuals(FLACContext *s, int32_t *decoded, int pred_order)
             for (; i < samples; i++)
                 *decoded++ = get_sbits_long(&gb, tmp);
         } else {
-            int real_limit = tmp ? (INT_MAX >> tmp) + 2 : INT_MAX;
+            int real_limit = (tmp > 1) ? (INT_MAX >> (tmp - 1)) + 2 : INT_MAX;
             for (; i < samples; i++) {
                 int v = get_sr_golomb_flac(&gb, tmp, real_limit, 1);
                 if (v == 0x80000000){