Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure
@ 2024-04-28 21:30 Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues Michael Niedermayer
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Fixes: CID1539100 Negative loop bound

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 fftools/opt_common.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fftools/opt_common.c b/fftools/opt_common.c
index 947a226d8d1..9d2d5184a08 100644
--- a/fftools/opt_common.c
+++ b/fftools/opt_common.c
@@ -724,10 +724,13 @@ int show_codecs(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
-static void print_codecs(int encoder)
+static int print_codecs(int encoder)
 {
     const AVCodecDescriptor **codecs;
-    unsigned i, nb_codecs = get_codecs_sorted(&codecs);
+    int i, nb_codecs = get_codecs_sorted(&codecs);
+
+    if (nb_codecs < 0)
+        return nb_codecs;
 
     printf("%s:\n"
            " V..... = Video\n"
@@ -762,18 +765,17 @@ static void print_codecs(int encoder)
         }
     }
     av_free(codecs);
+    return 0;
 }
 
 int show_decoders(void *optctx, const char *opt, const char *arg)
 {
-    print_codecs(0);
-    return 0;
+    return print_codecs(0);
 }
 
 int show_encoders(void *optctx, const char *opt, const char *arg)
 {
-    print_codecs(1);
-    return 0;
+    return print_codecs(1);
 }
 
 int show_bsfs(void *optctx, const char *opt, const char *arg)
-- 
2.43.2

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues
  2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
@ 2024-04-28 21:30 ` Michael Niedermayer
  2024-04-28 21:41   ` Lynne
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 3/5] avcodec/ac3_parser: Check init_get_bits8() for failure Michael Niedermayer
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Inspired by CID1465483 Unintentional integer overflow

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/aaccoder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
index 4ce54ca8867..6e5817e237b 100644
--- a/libavcodec/aaccoder.c
+++ b/libavcodec/aaccoder.c
@@ -178,6 +178,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
                         int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, ROUNDING), 13);
                         int len = av_log2(coef);
 
+                        av_assert2(len >= 4);
+
                         put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
                         put_sbits(pb, len, coef);
                     }
-- 
2.43.2

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH 3/5] avcodec/ac3_parser: Check init_get_bits8() for failure
  2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues Michael Niedermayer
@ 2024-04-28 21:30 ` Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector() Michael Niedermayer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Fixes: CID1420393 Unchecked return value

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ac3_parser.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/ac3_parser.c b/libavcodec/ac3_parser.c
index 4152fd4e018..4e0ba734818 100644
--- a/libavcodec/ac3_parser.c
+++ b/libavcodec/ac3_parser.c
@@ -204,7 +204,9 @@ int av_ac3_parse_header(const uint8_t *buf, size_t size,
     AC3HeaderInfo hdr;
     int err;
 
-    init_get_bits8(&gb, buf, size);
+    err = init_get_bits8(&gb, buf, size);
+    if (err < 0)
+        return AVERROR_INVALIDDATA;
     err = ff_ac3_parse_header(&gb, &hdr);
     if (err < 0)
         return AVERROR_INVALIDDATA;
-- 
2.43.2

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector()
  2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 3/5] avcodec/ac3_parser: Check init_get_bits8() for failure Michael Niedermayer
@ 2024-04-28 21:30 ` Michael Niedermayer
  2024-05-05  1:41   ` Michael Niedermayer
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure Michael Niedermayer
  2024-05-05  1:39 ` [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
  4 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Inspired-by: CID1473499 Uninitialized scalar variable

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/amrwbdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/amrwbdec.c b/libavcodec/amrwbdec.c
index 9d75b972fa7..21a730b835d 100644
--- a/libavcodec/amrwbdec.c
+++ b/libavcodec/amrwbdec.c
@@ -26,6 +26,7 @@
 
 #include "config.h"
 
+#include "libavutil/avassert.h"
 #include "libavutil/channel_layout.h"
 #include "libavutil/common.h"
 #include "libavutil/lfg.h"
@@ -554,6 +555,8 @@ static void decode_fixed_vector(float *fixed_vector, const uint16_t *pulse_hi,
             decode_6p_track(sig_pos[i], (int) pulse_lo[i] +
                            ((int) pulse_hi[i] << 11), 4, 1);
         break;
+    default:
+        av_assert2(0);
     }
 
     memset(fixed_vector, 0, sizeof(float) * AMRWB_SFR_SIZE);
-- 
2.43.2

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure
  2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
                   ` (2 preceding siblings ...)
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector() Michael Niedermayer
@ 2024-04-28 21:30 ` Michael Niedermayer
  2024-04-28 21:41   ` Lynne
  2024-04-29  6:52   ` Paul B Mahol
  2024-05-05  1:39 ` [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
  4 siblings, 2 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Fixes: CID1439569 Unchecked return value
Fixes: CID1439578 Unchecked return value

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/atrac9dec.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
index df68407af92..e375f46fd04 100644
--- a/libavcodec/atrac9dec.c
+++ b/libavcodec/atrac9dec.c
@@ -802,7 +802,9 @@ static int atrac9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     if (ret < 0)
         return ret;
 
-    init_get_bits8(&gb, avpkt->data, avpkt->size);
+    ret = init_get_bits8(&gb, avpkt->data, avpkt->size);
+    if (ret < 0)
+        return ret;
 
     for (int i = 0; i < frames; i++) {
         for (int j = 0; j < s->block_config->count; j++) {
@@ -922,7 +924,9 @@ static av_cold int atrac9_decode_init(AVCodecContext *avctx)
         return AVERROR_INVALIDDATA;
     }
 
-    init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
+    err = init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
+    if (err < 0)
+        return err;
 
     if (get_bits(&gb, 8) != 0xFE) {
         av_log(avctx, AV_LOG_ERROR, "Incorrect magic byte!\n");
-- 
2.43.2

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues Michael Niedermayer
@ 2024-04-28 21:41   ` Lynne
  2024-04-28 21:57     ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Lynne @ 2024-04-28 21:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Apr 28, 2024, 23:31 by michael@niedermayer.cc:

> Inspired by CID1465483 Unintentional integer overflow
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/aaccoder.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> index 4ce54ca8867..6e5817e237b 100644
> --- a/libavcodec/aaccoder.c
> +++ b/libavcodec/aaccoder.c
> @@ -178,6 +178,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
>  int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, ROUNDING), 13);
>  int len = av_log2(coef);
>  
> +                        av_assert2(len >= 4);
> +
>  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
>  put_sbits(pb, len, coef);
>  }
>

I'm not sure that's correct to do. Any specific cases where this happens?
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure Michael Niedermayer
@ 2024-04-28 21:41   ` Lynne
  2024-04-30 22:53     ` Michael Niedermayer
  2024-04-29  6:52   ` Paul B Mahol
  1 sibling, 1 reply; 15+ messages in thread
From: Lynne @ 2024-04-28 21:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Apr 28, 2024, 23:31 by michael@niedermayer.cc:

> Fixes: CID1439569 Unchecked return value
> Fixes: CID1439578 Unchecked return value
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/atrac9dec.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> index df68407af92..e375f46fd04 100644
> --- a/libavcodec/atrac9dec.c
> +++ b/libavcodec/atrac9dec.c
> @@ -802,7 +802,9 @@ static int atrac9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>  if (ret < 0)
>  return ret;
>  
> -    init_get_bits8(&gb, avpkt->data, avpkt->size);
> +    ret = init_get_bits8(&gb, avpkt->data, avpkt->size);
> +    if (ret < 0)
> +        return ret;
>  
>  for (int i = 0; i < frames; i++) {
>  for (int j = 0; j < s->block_config->count; j++) {
> @@ -922,7 +924,9 @@ static av_cold int atrac9_decode_init(AVCodecContext *avctx)
>  return AVERROR_INVALIDDATA;
>  }
>  
> -    init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
> +    err = init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
> +    if (err < 0)
> +        return err;
>  
>  if (get_bits(&gb, 8) != 0xFE) {
>  av_log(avctx, AV_LOG_ERROR, "Incorrect magic byte!\n");
>

This and ac3 lgtm
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues
  2024-04-28 21:41   ` Lynne
@ 2024-04-28 21:57     ` Michael Niedermayer
  2024-04-28 22:26       ` Lynne
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-28 21:57 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2084 bytes --]

On Sun, Apr 28, 2024 at 11:41:20PM +0200, Lynne wrote:
> Apr 28, 2024, 23:31 by michael@niedermayer.cc:
> 
> > Inspired by CID1465483 Unintentional integer overflow
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/aaccoder.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> > index 4ce54ca8867..6e5817e237b 100644
> > --- a/libavcodec/aaccoder.c
> > +++ b/libavcodec/aaccoder.c
> > @@ -178,6 +178,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
> >  int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, ROUNDING), 13);
> >  int len = av_log2(coef);
> >  
> > +                        av_assert2(len >= 4);
> > +
> >  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
> >  put_sbits(pb, len, coef);
> >  }
> >
> 
> I'm not sure that's correct to do. Any specific cases where this happens?

if len is 3 or less then put_bits will have a negative value or
undefined shift

coverity sasid this:
" overflow_before_widen: Potentially overflowing expression 1 << len - 4 + 1 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type BitBuf (64 bits, unsigned). To avoid overflow, cast 1 to type BitBuf."

So what coverity really said is that the expression could exeed 32bit _because_ its
used in 64bit context. Thats just stupid from coverity also teh clip above
limits this to 13 bit so i dont see how it can overflow in the "too large" direction
and i marked this one as false positive.

I wasnt 100% sure about the too small side, i tested it and its never too small
but coverity didnt claim it could be too small, so that question is outside the issue
So i added a assert in this patch.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues
  2024-04-28 21:57     ` Michael Niedermayer
@ 2024-04-28 22:26       ` Lynne
  2024-04-29 22:58         ` Michael Niedermayer
  0 siblings, 1 reply; 15+ messages in thread
From: Lynne @ 2024-04-28 22:26 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Apr 28, 2024, 23:57 by michael@niedermayer.cc:

> On Sun, Apr 28, 2024 at 11:41:20PM +0200, Lynne wrote:
>
>> Apr 28, 2024, 23:31 by michael@niedermayer.cc:
>>
>> > Inspired by CID1465483 Unintentional integer overflow
>> >
>> > Sponsored-by: Sovereign Tech Fund
>> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> > ---
>> >  libavcodec/aaccoder.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
>> > index 4ce54ca8867..6e5817e237b 100644
>> > --- a/libavcodec/aaccoder.c
>> > +++ b/libavcodec/aaccoder.c
>> > @@ -178,6 +178,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
>> >  int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, ROUNDING), 13);
>> >  int len = av_log2(coef);
>> > 
>> > +                        av_assert2(len >= 4);
>> > +
>> >  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
>> >  put_sbits(pb, len, coef);
>> >  }
>> >
>>
>> I'm not sure that's correct to do. Any specific cases where this happens?
>>
>
> if len is 3 or less then put_bits will have a negative value or
> undefined shift
>
> coverity sasid this:
> " overflow_before_widen: Potentially overflowing expression 1 << len - 4 + 1 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type BitBuf (64 bits, unsigned). To avoid overflow, cast 1 to type BitBuf."
>
> So what coverity really said is that the expression could exeed 32bit _because_ its
> used in 64bit context. Thats just stupid from coverity also teh clip above
> limits this to 13 bit so i dont see how it can overflow in the "too large" direction
> and i marked this one as false positive.
>
> I wasnt 100% sure about the too small side, i tested it and its never too small
> but coverity didnt claim it could be too small, so that question is outside the issue
> So i added a assert in this patch.
>

Could you instead use 64-bit arithmetic for `coef`?
Such a general assert here could result in false alarms, like the AAC
encoder suffered from before, which would make development harder.
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure Michael Niedermayer
  2024-04-28 21:41   ` Lynne
@ 2024-04-29  6:52   ` Paul B Mahol
  2024-04-29 23:30     ` Michael Niedermayer
  1 sibling, 1 reply; 15+ messages in thread
From: Paul B Mahol @ 2024-04-29  6:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Sun, Apr 28, 2024 at 11:31 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: CID1439569 Unchecked return value
> Fixes: CID1439578 Unchecked return value
>
> Sponsored-by: Sovereign Tech Fund
>

And they paid you for this low effort work!

Applause!



> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/atrac9dec.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> index df68407af92..e375f46fd04 100644
> --- a/libavcodec/atrac9dec.c
> +++ b/libavcodec/atrac9dec.c
> @@ -802,7 +802,9 @@ static int atrac9_decode_frame(AVCodecContext *avctx,
> AVFrame *frame,
>      if (ret < 0)
>          return ret;
>
> -    init_get_bits8(&gb, avpkt->data, avpkt->size);
> +    ret = init_get_bits8(&gb, avpkt->data, avpkt->size);
> +    if (ret < 0)
> +        return ret;
>
>      for (int i = 0; i < frames; i++) {
>          for (int j = 0; j < s->block_config->count; j++) {
> @@ -922,7 +924,9 @@ static av_cold int atrac9_decode_init(AVCodecContext
> *avctx)
>          return AVERROR_INVALIDDATA;
>      }
>
> -    init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
> +    err = init_get_bits8(&gb, avctx->extradata + 4,
> avctx->extradata_size);
> +    if (err < 0)
> +        return err;
>
>      if (get_bits(&gb, 8) != 0xFE) {
>          av_log(avctx, AV_LOG_ERROR, "Incorrect magic byte!\n");
> --
> 2.43.2
>
> _______________________________________________
> 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".
>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues
  2024-04-28 22:26       ` Lynne
@ 2024-04-29 22:58         ` Michael Niedermayer
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-29 22:58 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2713 bytes --]

On Mon, Apr 29, 2024 at 12:26:24AM +0200, Lynne wrote:
> Apr 28, 2024, 23:57 by michael@niedermayer.cc:
> 
> > On Sun, Apr 28, 2024 at 11:41:20PM +0200, Lynne wrote:
> >
> >> Apr 28, 2024, 23:31 by michael@niedermayer.cc:
> >>
> >> > Inspired by CID1465483 Unintentional integer overflow
> >> >
> >> > Sponsored-by: Sovereign Tech Fund
> >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >> > ---
> >> >  libavcodec/aaccoder.c | 2 ++
> >> >  1 file changed, 2 insertions(+)
> >> >
> >> > diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> >> > index 4ce54ca8867..6e5817e237b 100644
> >> > --- a/libavcodec/aaccoder.c
> >> > +++ b/libavcodec/aaccoder.c
> >> > @@ -178,6 +178,8 @@ static av_always_inline float quantize_and_encode_band_cost_template(
> >> >  int coef = av_clip_uintp2(quant(fabsf(in[i+j]), Q, ROUNDING), 13);
> >> >  int len = av_log2(coef);
> >> > 
> >> > +                        av_assert2(len >= 4);
> >> > +
> >> >  put_bits(pb, len - 4 + 1, (1 << (len - 4 + 1)) - 2);
> >> >  put_sbits(pb, len, coef);
> >> >  }
> >> >
> >>
> >> I'm not sure that's correct to do. Any specific cases where this happens?
> >>
> >
> > if len is 3 or less then put_bits will have a negative value or
> > undefined shift
> >
> > coverity sasid this:
> > " overflow_before_widen: Potentially overflowing expression 1 << len - 4 + 1 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type BitBuf (64 bits, unsigned). To avoid overflow, cast 1 to type BitBuf."
> >
> > So what coverity really said is that the expression could exeed 32bit _because_ its
> > used in 64bit context. Thats just stupid from coverity also teh clip above
> > limits this to 13 bit so i dont see how it can overflow in the "too large" direction
> > and i marked this one as false positive.
> >
> > I wasnt 100% sure about the too small side, i tested it and its never too small
> > but coverity didnt claim it could be too small, so that question is outside the issue
> > So i added a assert in this patch.
> >
> 
> Could you instead use 64-bit arithmetic for `coef`?
> Such a general assert here could result in false alarms, like the AAC
> encoder suffered from before, which would make development harder.

If you dont like the assert, then its best to just drop the patch.
It doesnt fix anything, it was just intended to clarify that the
case of len < 4 doesnt occur. The coverity issue is a false positive

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure
  2024-04-29  6:52   ` Paul B Mahol
@ 2024-04-29 23:30     ` Michael Niedermayer
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-29 23:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1274 bytes --]

On Mon, Apr 29, 2024 at 08:52:32AM +0200, Paul B Mahol wrote:
> On Sun, Apr 28, 2024 at 11:31 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Fixes: CID1439569 Unchecked return value
> > Fixes: CID1439578 Unchecked return value
> >
> > Sponsored-by: Sovereign Tech Fund
> >
> 
> And they paid you for this low effort work!

not yet, i have over 600 coverity issues to go over, ATM coverity lists 677
outstanding.

some are in fact low effort, I wish all where low effort sadly thats not the case

Also these issues are open since many years (these 2 here since 2018)
apparently they where too high effort for anyone to deal with them before

also you and everyone is very welcome to submit a proposal / SoW
for any maintaince work. Low effort, High effort, anything ffmpeg
related. I presume there will be future rounds from STF


> 
> Applause!

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure
  2024-04-28 21:41   ` Lynne
@ 2024-04-30 22:53     ` Michael Niedermayer
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-04-30 22:53 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1736 bytes --]

On Sun, Apr 28, 2024 at 11:41:48PM +0200, Lynne wrote:
> Apr 28, 2024, 23:31 by michael@niedermayer.cc:
> 
> > Fixes: CID1439569 Unchecked return value
> > Fixes: CID1439578 Unchecked return value
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/atrac9dec.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/atrac9dec.c b/libavcodec/atrac9dec.c
> > index df68407af92..e375f46fd04 100644
> > --- a/libavcodec/atrac9dec.c
> > +++ b/libavcodec/atrac9dec.c
> > @@ -802,7 +802,9 @@ static int atrac9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
> >  if (ret < 0)
> >  return ret;
> >  
> > -    init_get_bits8(&gb, avpkt->data, avpkt->size);
> > +    ret = init_get_bits8(&gb, avpkt->data, avpkt->size);
> > +    if (ret < 0)
> > +        return ret;
> >  
> >  for (int i = 0; i < frames; i++) {
> >  for (int j = 0; j < s->block_config->count; j++) {
> > @@ -922,7 +924,9 @@ static av_cold int atrac9_decode_init(AVCodecContext *avctx)
> >  return AVERROR_INVALIDDATA;
> >  }
> >  
> > -    init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
> > +    err = init_get_bits8(&gb, avctx->extradata + 4, avctx->extradata_size);
> > +    if (err < 0)
> > +        return err;
> >  
> >  if (get_bits(&gb, 8) != 0xFE) {
> >  av_log(avctx, AV_LOG_ERROR, "Incorrect magic byte!\n");
> >
> 
> This and ac3 lgtm

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure
  2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
                   ` (3 preceding siblings ...)
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure Michael Niedermayer
@ 2024-05-05  1:39 ` Michael Niedermayer
  4 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-05-05  1:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 603 bytes --]

On Sun, Apr 28, 2024 at 11:30:48PM +0200, Michael Niedermayer wrote:
> Fixes: CID1539100 Negative loop bound
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  fftools/opt_common.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector()
  2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector() Michael Niedermayer
@ 2024-05-05  1:41   ` Michael Niedermayer
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Niedermayer @ 2024-05-05  1:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 483 bytes --]

On Sun, Apr 28, 2024 at 11:30:51PM +0200, Michael Niedermayer wrote:
> Inspired-by: CID1473499 Uninitialized scalar variable
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/amrwbdec.c | 3 +++
>  1 file changed, 3 insertions(+)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-05-05  1:41 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-28 21:30 [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer
2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 2/5] avcodec/aaccoder: assert that escape case len is not causing issues Michael Niedermayer
2024-04-28 21:41   ` Lynne
2024-04-28 21:57     ` Michael Niedermayer
2024-04-28 22:26       ` Lynne
2024-04-29 22:58         ` Michael Niedermayer
2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 3/5] avcodec/ac3_parser: Check init_get_bits8() for failure Michael Niedermayer
2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 4/5] avcodec/amrwbdec: assert mode to be valid in decode_fixed_vector() Michael Niedermayer
2024-05-05  1:41   ` Michael Niedermayer
2024-04-28 21:30 ` [FFmpeg-devel] [PATCH 5/5] avcodec/atrac9dec: Check init_get_bits8() for failure Michael Niedermayer
2024-04-28 21:41   ` Lynne
2024-04-30 22:53     ` Michael Niedermayer
2024-04-29  6:52   ` Paul B Mahol
2024-04-29 23:30     ` Michael Niedermayer
2024-05-05  1:39 ` [FFmpeg-devel] [PATCH 1/5] tools/opt_common: Check for malloc failure Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git