| Message ID | 20170820011908.26188-1-vitalybuka@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Aug 20, 2017 at 3:19 AM, Vitaly Buka <vitalybuka-at-google.com@ffmpeg.org> wrote: > Signed integer overflow is undefined behavior. > Detected with clang and -fsanitize=signed-integer-overflow > > Signed-off-by: Vitaly Buka <vitalybuka@google.com> > --- > libavcodec/utils.c | 2 +- > libavformat/aviobuf.c | 2 ++ > libavformat/mov.c | 2 +- > 3 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 1336e921c9..024dc1f3e2 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -971,7 +971,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > } > > if (!avctx->rc_initial_buffer_occupancy) > - avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size * 3 / 4; > + avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size * 3ll / 4; > > Is "ll" portable? We seem to use the INT64_C macro in other places. - Hendrik
Looks like libavcodec/ has more LL or ll than INT64_C. Should I update the patch? On Sat, Aug 19, 2017 at 11:35 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Sun, Aug 20, 2017 at 3:19 AM, Vitaly Buka > <vitalybuka-at-google.com@ffmpeg.org> wrote: > > Signed integer overflow is undefined behavior. > > Detected with clang and -fsanitize=signed-integer-overflow > > > > Signed-off-by: Vitaly Buka <vitalybuka@google.com> > > --- > > libavcodec/utils.c | 2 +- > > libavformat/aviobuf.c | 2 ++ > > libavformat/mov.c | 2 +- > > 3 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > > index 1336e921c9..024dc1f3e2 100644 > > --- a/libavcodec/utils.c > > +++ b/libavcodec/utils.c > > @@ -971,7 +971,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > } > > > > if (!avctx->rc_initial_buffer_occupancy) > > - avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size > * 3 / 4; > > + avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size > * 3ll / 4; > > > > > > Is "ll" portable? We seem to use the INT64_C macro in other places. > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 8/20/2017 4:10 AM, Vitaly Buka wrote: > Looks like libavcodec/ has more LL or ll than INT64_C. > Should I update the patch? IMO yes. LL is known to work whereas ll might not in some fringe setups (we have a few of those running FATE). > > On Sat, Aug 19, 2017 at 11:35 PM, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > >> On Sun, Aug 20, 2017 at 3:19 AM, Vitaly Buka >> <vitalybuka-at-google.com@ffmpeg.org> wrote: >>> Signed integer overflow is undefined behavior. >>> Detected with clang and -fsanitize=signed-integer-overflow >>> >>> Signed-off-by: Vitaly Buka <vitalybuka@google.com> >>> --- >>> libavcodec/utils.c | 2 +- >>> libavformat/aviobuf.c | 2 ++ >>> libavformat/mov.c | 2 +- >>> 3 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >>> index 1336e921c9..024dc1f3e2 100644 >>> --- a/libavcodec/utils.c >>> +++ b/libavcodec/utils.c >>> @@ -971,7 +971,7 @@ FF_ENABLE_DEPRECATION_WARNINGS >>> } >>> >>> if (!avctx->rc_initial_buffer_occupancy) >>> - avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size >> * 3 / 4; >>> + avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size >> * 3ll / 4; >>> >>> >> >> Is "ll" portable? We seem to use the INT64_C macro in other places. >> >> - Hendrik >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 1336e921c9..024dc1f3e2 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -971,7 +971,7 @@ FF_ENABLE_DEPRECATION_WARNINGS } if (!avctx->rc_initial_buffer_occupancy) - avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size * 3 / 4; + avctx->rc_initial_buffer_occupancy = avctx->rc_buffer_size * 3ll / 4; if (avctx->ticks_per_frame && avctx->time_base.num && avctx->ticks_per_frame > INT_MAX / avctx->time_base.num) { diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 7f4e740a33..ec21fc7d38 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -259,6 +259,8 @@ int64_t avio_seek(AVIOContext *s, int64_t offset, int whence) offset1 = pos + (s->buf_ptr - s->buffer); if (offset == 0) return offset1; + if (offset > INT64_MAX - offset1) + return AVERROR(EINVAL); offset += offset1; } if (offset < 0) diff --git a/libavformat/mov.c b/libavformat/mov.c index 522ce60c2d..a14c9f182b 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -5572,7 +5572,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) if (atom.size < 0) atom.size = INT64_MAX; - while (total_size + 8 <= atom.size && !avio_feof(pb)) { + while (total_size <= atom.size - 8 && !avio_feof(pb)) { int (*parse)(MOVContext*, AVIOContext*, MOVAtom) = NULL; a.size = atom.size; a.type=0;
Signed integer overflow is undefined behavior. Detected with clang and -fsanitize=signed-integer-overflow Signed-off-by: Vitaly Buka <vitalybuka@google.com> --- libavcodec/utils.c | 2 +- libavformat/aviobuf.c | 2 ++ libavformat/mov.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-)