On Thu, Feb 6, 2020 at 3:38 PM Michael Niedermayer <michae...@gmx.at> wrote:

> On Thu, Jan 30, 2020 at 11:23:07AM -0800, Dale Curtis wrote:
> > On Wed, Jan 29, 2020 at 10:23 PM Michael Niedermayer
> <mich...@niedermayer.cc>
> > wrote:
> >
> > > so i think it works but maybe ive missed something, for which values
> > > of e2_pts do you see a problem with e1_pts = INT64_MIN?
> > >
> >
> > For e1_pts = INT64_MIN and e2_pts >= 0 you end up with a negative int64_t
> > result for e2_pts - (uint64_t)e1_pts, so it's always < time_tolerance. If
> > that's what you intended, then sgtm.
>
> thats what the code would do if the elemnts where large enough to not
> overflow
> so that seems to match whats intended.
>
> Do you see some issue here ?
>

Whoops, sorry, I just realized my previous comment was rejected from the
list since I used the wrong e-mail address. My previous statement was
incorrect. The code you proposed correctly promotes a uint64_t and not an
int64_t when used in a conditional. Here's what I wrote that got lost:
"Actually, this was a test construction error on my part. In a conditional
this does evaluate to a uint64_t vs int64_t, so the comparison is valid.
I've attached your recommended patch. Thanks for your patience."

- dale
From c50d0a347fc779c71c07757d9cad8a7d56eb857b Mon Sep 17 00:00:00 2001
From: Dale Curtis <dalecur...@chromium.org>
Date: Tue, 28 Jan 2020 16:49:14 -0800
Subject: [PATCH] Fix undefined behavior in ff_configure_buffers_for_index()

When e2_pts == INT64_MIN and e1_pts >= 0 the calculation of
e2_pts - e1_pts will overflow an int64_t.

Signed-off-by: Dale Curtis <dalecur...@chromium.org>
---
 libavformat/utils.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index e22ca7cab8..81ea239a66 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2108,6 +2108,8 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
     //We could use URLProtocol flags here but as many user applications do not use URLProtocols this would be unreliable
     const char *proto = avio_find_protocol_name(s->url);
 
+    av_assert0(time_tolerance >= 0);
+
     if (!proto) {
         av_log(s, AV_LOG_INFO,
                "Protocol name not provided, cannot determine if input is local or "
@@ -2135,7 +2137,7 @@ void ff_configure_buffers_for_index(AVFormatContext *s, int64_t time_tolerance)
                 for (; i2 < st2->nb_index_entries; i2++) {
                     AVIndexEntry *e2 = &st2->index_entries[i2];
                     int64_t e2_pts = av_rescale_q(e2->timestamp, st2->time_base, AV_TIME_BASE_Q);
-                    if (e2_pts - e1_pts < time_tolerance)
+                    if (e2_pts < e1_pts || e2_pts - (uint64_t)e1_pts < time_tolerance)
                         continue;
                     pos_delta = FFMAX(pos_delta, e1->pos - e2->pos);
                     break;
-- 
2.25.0.341.g760bfbb309-goog

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to