[FFmpeg-devel] [PATCH v2 68/69] avcodec/mpegvideo_dec: Remove potentially UB always-true checks

[Andreas Rheinhardt]

ff_mpeg_update_thread_context() currently checks for whether
the source (current|last|next)_picture_ptr points into the
src context's picture array by performing a pointer comparison.

Yet pointer comparisons are only legal when the pointers point
into the same array object (or one past the last element);
otherwise they are undefined behaviour that happen to work
(at least with a flat address space).

In this case this code is moreover a remnant of the time
when the H.264 decoder used H.264 (see the commit message
of d9df93efbf59b1dc8b013d174ca4ad9c634c28f7); the current decoders
never set these pointers to anything outside of the picture array
(except NULL). So remove these checks.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
---
 libavcodec/mpegvideo.h     | 3 +++
 libavcodec/mpegvideo_dec.c | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index a65c23f1d1..af1d9af2bd 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -142,6 +142,9 @@ typedef struct MPVContext {
      */
     Picture current_picture;    ///< buffer to store the decompressed current picture
 
+    /* The following three pointers must be either NULL or point
+     * to a picture in the main picture buffer (i.e. picture)
+     * for users of mpegvideodec. */
     Picture *last_picture_ptr;     ///< pointer to the previous picture.
     Picture *next_picture_ptr;     ///< pointer to the next picture (for bidir pred)
     Picture *current_picture_ptr;  ///< pointer to the current picture
diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c
index 8927a0a21b..137b47efa7 100644
--- a/libavcodec/mpegvideo_dec.c
+++ b/libavcodec/mpegvideo_dec.c
@@ -74,6 +74,9 @@ int ff_mpeg_update_thread_context(AVCodecContext *dst,
         s->avctx                 = dst;
         s->parent_ctx            = m;
         s->private_ctx           = private_ctx;
+        s->current_picture_ptr   = NULL;
+        s->next_picture_ptr      = NULL;
+        s->last_picture_ptr      = NULL;
         s->bitstream_buffer      = NULL;
         s->bitstream_buffer_size = s->allocated_bitstream_buffer_size = 0;
 
@@ -133,9 +136,7 @@ do {\
     UPDATE_PICTURE(next_picture);
 
 #define REBASE_PICTURE(pic, new_ctx, old_ctx)                                 \
-    ((pic && pic >= old_ctx->picture &&                                       \
-      pic < old_ctx->picture + MAX_PICTURE_COUNT) ?                           \
-        &new_ctx->picture[pic - old_ctx->picture] : NULL)
+    ((pic) ? &(new_ctx)->picture[(pic) - (old_ctx)->picture] : NULL)
 
     s->last_picture_ptr    = REBASE_PICTURE(s1->last_picture_ptr,    s, s1);
     s->current_picture_ptr = REBASE_PICTURE(s1->current_picture_ptr, s, s1);
-- 
2.32.0