| Message ID | CADbEz-gm8p5WcaNFBTe25XvevA9k8VMkXfmzy4LoPX+iB9SD-A@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Nov 16, 2017 at 12:41:32PM -0800, Nick Lewycky wrote: > I initially discovered a signed integer overflow on this line. Since > this value is updated in multiple threads, I use an atomic update and > as it happens atomic addition is defined to wrap around. However, > there's still a potential bug in that the error_count may wrap around > and equal zero again causing problems down the line. > > --- > libavcodec/mpeg12dec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c > index d5bc5f21b2..b7c3b5106e 100644 > --- a/libavcodec/mpeg12dec.c > +++ b/libavcodec/mpeg12dec.c > @@ -28,6 +28,7 @@ > #define UNCHECKED_BITSTREAM_READER 1 > #include <inttypes.h> > > +#include "libavutil/atomic.h" > #include "libavutil/attributes.h" > #include "libavutil/imgutils.h" > #include "libavutil/internal.h" > @@ -2476,7 +2477,7 @@ static int decode_chunks(AVCodecContext *avctx, > AVFrame *picture, > &s2->thread_context[0], NULL, > s->slice_count, sizeof(void *)); > for (i = 0; i < s->slice_count; i++) > - s2->er.error_count += > s2->thread_context[i]->er.error_count; > + > avpriv_atomic_int_add_and_fetch(&s2->er.error_count, > s2->thread_context[i]->er.error_count); > } This patch is corrupted by newlines [...]
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index d5bc5f21b2..b7c3b5106e 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -28,6 +28,7 @@ #define UNCHECKED_BITSTREAM_READER 1 #include <inttypes.h> +#include "libavutil/atomic.h" #include "libavutil/attributes.h" #include "libavutil/imgutils.h" #include "libavutil/internal.h" @@ -2476,7 +2477,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture, &s2->thread_context[0], NULL, s->slice_count, sizeof(void *)); for (i = 0; i < s->slice_count; i++) - s2->er.error_count += s2->thread_context[i]->er.error_count; + avpriv_atomic_int_add_and_fetch(&s2->er.error_count,
I initially discovered a signed integer overflow on this line. Since this value is updated in multiple threads, I use an atomic update and as it happens atomic addition is defined to wrap around. However, there's still a potential bug in that the error_count may wrap around and equal zero again causing problems down the line. --- libavcodec/mpeg12dec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) s2->thread_context[i]->er.error_count); } ret = slice_end(avctx, picture);