diff mbox series

[FFmpeg-devel,3/5] avcodec/dcadec: Do not explode EAGAIN

Message ID 20230918223534.23567-3-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/5] tools/target_dec_fuzzer: Adjust wmapro threshold | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Michael Niedermayer Sept. 18, 2023, 10:35 p.m. UTC
Fixes: out of array access
Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DCA_fuzzer-6041088751960064

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

Comments

James Almer Sept. 18, 2023, 11:24 p.m. UTC | #1
On 9/18/2023 7:35 PM, Michael Niedermayer wrote:
> Fixes: out of array access
> Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DCA_fuzzer-6041088751960064
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/dcadec.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
> index 3e3e3053bbe..070a9ae094d 100644
> --- a/libavcodec/dcadec.c
> +++ b/libavcodec/dcadec.c
> @@ -221,7 +221,7 @@ static int dcadec_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>                       && (prev_packet & DCA_PACKET_XLL)
>                       && (s->packet & DCA_PACKET_CORE))
>                       s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> -                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
> +                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE) && ret != AVERROR(EAGAIN))
>                       return ret;
>               } else {
>                   s->packet |= DCA_PACKET_XLL;

Maybe instead do

> diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
> index 3e3e3053bb..3926115f21 100644
> --- a/libavcodec/dcadec.c
> +++ b/libavcodec/dcadec.c
> @@ -217,11 +217,10 @@ static int dcadec_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>          if (asset && (asset->extension_mask & DCA_EXSS_XLL)) {
>              if ((ret = ff_dca_xll_parse(&s->xll, input, asset)) < 0) {
>                  // Conceal XLL synchronization error
> -                if (ret == AVERROR(EAGAIN)
> -                    && (prev_packet & DCA_PACKET_XLL)
> -                    && (s->packet & DCA_PACKET_CORE))
> -                    s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> -                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
> +                if (ret == AVERROR(EAGAIN) {
> +                    if ((prev_packet & DCA_PACKET_XLL) && (s->packet & DCA_PACKET_CORE))
> +                        s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> +                } else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
>                      return ret;
>              } else {
>                  s->packet |= DCA_PACKET_XLL;

So EAGAIN is handled in one place.

LGTM either way.
Michael Niedermayer Sept. 19, 2023, 6:18 p.m. UTC | #2
On Mon, Sep 18, 2023 at 08:24:25PM -0300, James Almer wrote:
> On 9/18/2023 7:35 PM, Michael Niedermayer wrote:
> > Fixes: out of array access
> > Fixes: 62164/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DCA_fuzzer-6041088751960064
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/dcadec.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
> > index 3e3e3053bbe..070a9ae094d 100644
> > --- a/libavcodec/dcadec.c
> > +++ b/libavcodec/dcadec.c
> > @@ -221,7 +221,7 @@ static int dcadec_decode_frame(AVCodecContext *avctx, AVFrame *frame,
> >                       && (prev_packet & DCA_PACKET_XLL)
> >                       && (s->packet & DCA_PACKET_CORE))
> >                       s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> > -                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
> > +                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE) && ret != AVERROR(EAGAIN))
> >                       return ret;
> >               } else {
> >                   s->packet |= DCA_PACKET_XLL;
> 
> Maybe instead do
> 
> > diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
> > index 3e3e3053bb..3926115f21 100644
> > --- a/libavcodec/dcadec.c
> > +++ b/libavcodec/dcadec.c
> > @@ -217,11 +217,10 @@ static int dcadec_decode_frame(AVCodecContext *avctx, AVFrame *frame,
> >          if (asset && (asset->extension_mask & DCA_EXSS_XLL)) {
> >              if ((ret = ff_dca_xll_parse(&s->xll, input, asset)) < 0) {
> >                  // Conceal XLL synchronization error
> > -                if (ret == AVERROR(EAGAIN)
> > -                    && (prev_packet & DCA_PACKET_XLL)
> > -                    && (s->packet & DCA_PACKET_CORE))
> > -                    s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> > -                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
> > +                if (ret == AVERROR(EAGAIN) {
> > +                    if ((prev_packet & DCA_PACKET_XLL) && (s->packet & DCA_PACKET_CORE))
> > +                        s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
> > +                } else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
> >                      return ret;
> >              } else {
> >                  s->packet |= DCA_PACKET_XLL;
> 
> So EAGAIN is handled in one place.

ok will apply with you as author as its your change now

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
index 3e3e3053bbe..070a9ae094d 100644
--- a/libavcodec/dcadec.c
+++ b/libavcodec/dcadec.c
@@ -221,7 +221,7 @@  static int dcadec_decode_frame(AVCodecContext *avctx, AVFrame *frame,
                     && (prev_packet & DCA_PACKET_XLL)
                     && (s->packet & DCA_PACKET_CORE))
                     s->packet |= DCA_PACKET_XLL | DCA_PACKET_RECOVERY;
-                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE))
+                else if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & AV_EF_EXPLODE) && ret != AVERROR(EAGAIN))
                     return ret;
             } else {
                 s->packet |= DCA_PACKET_XLL;