[FFmpeg-devel] [PATCH 25/57] avcodec/me_cmp, motion_est: Sanitize permissible cmp_funcs

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Several of the potential choices of comparison functions
need an initialized MpegEncContext (initialized for encoding,
not only ff_mpv_common_init()) or they crash when called.
Modify ff_set_cmp() to check for this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/dvenc.c         |   2 +-
 libavcodec/me_cmp.c        | 116 +++++++++++++++++--------------------
 libavcodec/me_cmp.h        |   9 ++-
 libavcodec/motion_est.c    |  11 ++--
 libavcodec/motion_est.h    |   2 +-
 libavcodec/mpegvideo_enc.c |   6 +-
 libavcodec/snowenc.c       |   2 +-
 libavcodec/svq1enc.c       |   2 +-
 8 files changed, 73 insertions(+), 77 deletions(-)

diff --git a/libavcodec/dvenc.c b/libavcodec/dvenc.c
index 784340744e..f2221d0b94 100644
--- a/libavcodec/dvenc.c
+++ b/libavcodec/dvenc.c
@@ -100,7 +100,7 @@ static av_cold int dvvideo_encode_init(AVCodecContext *avctx)
 
         memset(&mecc,0, sizeof(mecc));
         ff_me_cmp_init(&mecc, avctx);
-        ret = ff_set_cmp(&mecc, ildct_cmp, avctx->ildct_cmp);
+        ret = ff_set_cmp(&mecc, ildct_cmp, avctx->ildct_cmp, 0);
         if (ret < 0)
             return ret;
         if (!ildct_cmp[5])
diff --git a/libavcodec/me_cmp.c b/libavcodec/me_cmp.c
index cff8936e44..dfc351d7ae 100644
--- a/libavcodec/me_cmp.c
+++ b/libavcodec/me_cmp.c
@@ -20,6 +20,8 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <stddef.h>
+
 #include "libavutil/attributes.h"
 #include "libavutil/internal.h"
 #include "libavutil/mem_internal.h"
@@ -473,74 +475,60 @@ static int zero_cmp(MpegEncContext *s, const uint8_t *a, const uint8_t *b,
     return 0;
 }
 
-int ff_set_cmp(const MECmpContext *c, me_cmp_func *cmp, int type)
+av_cold int ff_set_cmp(const MECmpContext *c, me_cmp_func *cmp, int type, int mpvenc)
 {
-    int ret = 0;
-    int i;
-
-    memset(cmp, 0, sizeof(void *) * 6);
-
-    for (i = 0; i < 6; i++) {
-        switch (type & 0xFF) {
-        case FF_CMP_SAD:
-            cmp[i] = c->sad[i];
-            break;
-        case FF_CMP_MEDIAN_SAD:
-            cmp[i] = c->median_sad[i];
-            break;
-        case FF_CMP_SATD:
-            cmp[i] = c->hadamard8_diff[i];
-            break;
-        case FF_CMP_SSE:
-            cmp[i] = c->sse[i];
-            break;
-        case FF_CMP_DCT:
-            cmp[i] = c->dct_sad[i];
-            break;
-        case FF_CMP_DCT264:
-            cmp[i] = c->dct264_sad[i];
-            break;
-        case FF_CMP_DCTMAX:
-            cmp[i] = c->dct_max[i];
-            break;
-        case FF_CMP_PSNR:
-            cmp[i] = c->quant_psnr[i];
-            break;
-        case FF_CMP_BIT:
-            cmp[i] = c->bit[i];
-            break;
-        case FF_CMP_RD:
-            cmp[i] = c->rd[i];
-            break;
-        case FF_CMP_VSAD:
-            cmp[i] = c->vsad[i];
-            break;
-        case FF_CMP_VSSE:
-            cmp[i] = c->vsse[i];
-            break;
-        case FF_CMP_ZERO:
-            cmp[i] = zero_cmp;
-            break;
-        case FF_CMP_NSSE:
-            cmp[i] = c->nsse[i];
-            break;
-#if CONFIG_DWT
-        case FF_CMP_W53:
-            cmp[i]= c->w53[i];
-            break;
-        case FF_CMP_W97:
-            cmp[i]= c->w97[i];
-            break;
+#define ENTRY(CMP_FLAG, ARRAY, MPVENC_ONLY)          \
+    [FF_CMP_ ## CMP_FLAG] = {                        \
+        .offset    = offsetof(MECmpContext, ARRAY),  \
+        .mpv_only  = MPVENC_ONLY,                    \
+        .available = 1,                              \
+    }
+    static const struct {
+        char available;
+        char mpv_only;
+        uint16_t offset;
+    } cmp_func_list[] = {
+        ENTRY(SAD,        sad,            0),
+        ENTRY(SSE,        sse,            0),
+        ENTRY(SATD,       hadamard8_diff, 0),
+        ENTRY(DCT,        dct_sad,        1),
+        ENTRY(PSNR,       quant_psnr,     1),
+        ENTRY(BIT,        bit,            1),
+        ENTRY(RD,         rd,             1),
+        ENTRY(VSAD,       vsad,           0),
+        ENTRY(VSSE,       vsse,           0),
+        ENTRY(NSSE,       nsse,           0),
+#if CONFIG_SNOW_DECODER || CONFIG_SNOW_ENCODER
+        ENTRY(W53,        w53,            0),
+        ENTRY(W97,        w97,            0),
 #endif
-        default:
-            av_log(NULL, AV_LOG_ERROR,
-                   "invalid cmp function selection\n");
-            ret = -1;
-            break;
-        }
+        ENTRY(DCTMAX,     dct_max,        1),
+#if CONFIG_GPL
+        ENTRY(DCT264,     dct264_sad,     1),
+#endif
+        ENTRY(MEDIAN_SAD, median_sad,     0),
+    };
+    const me_cmp_func *me_cmp_func_array;
+
+    type &= 0xFF;
+
+    if (type == FF_CMP_ZERO) {
+        for (int i = 0; i < 6; i++)
+            cmp[i] = zero_cmp;
+        return 0;
+    }
+    if (type > FF_ARRAY_ELEMS(cmp_func_list) ||
+        !cmp_func_list[type].available ||
+        !mpvenc && cmp_func_list[type].mpv_only) {
+        av_log(NULL, AV_LOG_ERROR,
+               "invalid cmp function selection\n");
+        return AVERROR(EINVAL);
     }
+    me_cmp_func_array = (const me_cmp_func*)(((const char*)c) + cmp_func_list[type].offset);
+    for (int i = 0; i < 6; i++)
+        cmp[i] = me_cmp_func_array[i];
 
-    return ret;
+    return 0;
 }
 
 #define BUTTERFLY2(o1, o2, i1, i2)              \
diff --git a/libavcodec/me_cmp.h b/libavcodec/me_cmp.h
index b9abc7fb8e..9053327c4c 100644
--- a/libavcodec/me_cmp.h
+++ b/libavcodec/me_cmp.h
@@ -83,7 +83,14 @@ void ff_me_cmp_init_riscv(MECmpContext *c, AVCodecContext *avctx);
 void ff_me_cmp_init_x86(MECmpContext *c, AVCodecContext *avctx);
 void ff_me_cmp_init_mips(MECmpContext *c, AVCodecContext *avctx);
 
-int ff_set_cmp(const MECmpContext *c, me_cmp_func *cmp, int type);
+/**
+ * Fill the function pointer array cmp[6] with me_cmp_funcs from
+ * c based upon type. If mpvenc is not set, an error is returned
+ * if the type of comparison functions requires an initialized
+ * MpegEncContext.
+ */
+int ff_set_cmp(const MECmpContext *c, me_cmp_func *cmp,
+               int type, int mpvenc);
 
 void ff_dsputil_init_dwt(MECmpContext *c);
 
diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c
index b29d0c6d96..13f3d8040e 100644
--- a/libavcodec/motion_est.c
+++ b/libavcodec/motion_est.c
@@ -305,7 +305,8 @@ static int zero_cmp(MpegEncContext *s, const uint8_t *a, const uint8_t *b,
 static void zero_hpel(uint8_t *a, const uint8_t *b, ptrdiff_t stride, int h){
 }
 
-av_cold int ff_me_init(MotionEstContext *c, AVCodecContext *avctx, const MECmpContext *mecc)
+av_cold int ff_me_init(MotionEstContext *c, AVCodecContext *avctx,
+                       const MECmpContext *mecc, int mpvenc)
 {
     int cache_size = FFMIN(ME_MAP_SIZE>>ME_MAP_SHIFT, 1<<ME_MAP_SHIFT);
     int dia_size   = FFMAX(FFABS(avctx->dia_size) & 255, FFABS(avctx->pre_dia_size) & 255);
@@ -324,10 +325,10 @@ av_cold int ff_me_init(MotionEstContext *c, AVCodecContext *avctx, const MECmpCo
     if (cache_size < 2 * dia_size)
         av_log(avctx, AV_LOG_INFO, "ME_MAP size may be a little small for the selected diamond size\n");
 
-    ret  = ff_set_cmp(mecc, c->me_pre_cmp, avctx->me_pre_cmp);
-    ret |= ff_set_cmp(mecc, c->me_cmp,     avctx->me_cmp);
-    ret |= ff_set_cmp(mecc, c->me_sub_cmp, avctx->me_sub_cmp);
-    ret |= ff_set_cmp(mecc, c->mb_cmp,     avctx->mb_cmp);
+    ret  = ff_set_cmp(mecc, c->me_pre_cmp, avctx->me_pre_cmp, mpvenc);
+    ret |= ff_set_cmp(mecc, c->me_cmp,     avctx->me_cmp,     mpvenc);
+    ret |= ff_set_cmp(mecc, c->me_sub_cmp, avctx->me_sub_cmp, mpvenc);
+    ret |= ff_set_cmp(mecc, c->mb_cmp,     avctx->mb_cmp,     mpvenc);
     if (ret < 0)
         return ret;
 
diff --git a/libavcodec/motion_est.h b/libavcodec/motion_est.h
index 5547aeb8bc..243b73ff4e 100644
--- a/libavcodec/motion_est.h
+++ b/libavcodec/motion_est.h
@@ -115,7 +115,7 @@ static inline int ff_h263_round_chroma(int x)
  * Performs one-time initialization of the MotionEstContext.
  */
 int ff_me_init(MotionEstContext *c, struct AVCodecContext *avctx,
-               const struct MECmpContext *mecc);
+               const struct MECmpContext *mecc, int mpvenc);
 
 void ff_me_init_pic(struct MpegEncContext *s);
 
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 6059bdee11..8022fe474a 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -312,15 +312,15 @@ static av_cold int me_cmp_init(MpegEncContext *s, AVCodecContext *avctx)
     int ret;
 
     ff_me_cmp_init(&s->mecc, avctx);
-    ret = ff_me_init(&s->me, avctx, &s->mecc);
+    ret = ff_me_init(&s->me, avctx, &s->mecc, 1);
     if (ret < 0)
         return ret;
-    ret = ff_set_cmp(&s->mecc, me_cmp, s->frame_skip_cmp);
+    ret = ff_set_cmp(&s->mecc, me_cmp, s->frame_skip_cmp, 1);
     if (ret < 0)
         return ret;
     s->frame_skip_cmp_fn = me_cmp[1];
     if (avctx->flags & AV_CODEC_FLAG_INTERLACED_DCT) {
-        ret = ff_set_cmp(&s->mecc, me_cmp, avctx->ildct_cmp);
+        ret = ff_set_cmp(&s->mecc, me_cmp, avctx->ildct_cmp, 1);
         if (ret < 0)
             return ret;
         if (!me_cmp[0] || !me_cmp[4])
diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c
index 9d64b1efe2..819a7933fe 100644
--- a/libavcodec/snowenc.c
+++ b/libavcodec/snowenc.c
@@ -217,7 +217,7 @@ static av_cold int encode_init(AVCodecContext *avctx)
     mcf(12,12)
 
     ff_me_cmp_init(&enc->mecc, avctx);
-    ret = ff_me_init(&enc->m.me, avctx, &enc->mecc);
+    ret = ff_me_init(&enc->m.me, avctx, &enc->mecc, 0);
     if (ret < 0)
         return ret;
     ff_mpegvideoencdsp_init(&enc->mpvencdsp, avctx);
diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
index ceb8bf83c2..5413508217 100644
--- a/libavcodec/svq1enc.c
+++ b/libavcodec/svq1enc.c
@@ -589,7 +589,7 @@ static av_cold int svq1_encode_init(AVCodecContext *avctx)
 
     ff_hpeldsp_init(&s->hdsp, avctx->flags);
     ff_me_cmp_init(&s->mecc, avctx);
-    ret = ff_me_init(&s->m.me, avctx, &s->mecc);
+    ret = ff_me_init(&s->m.me, avctx, &s->mecc, 0);
     if (ret < 0)
         return ret;
     ff_mpegvideoencdsp_init(&s->m.mpvencdsp, avctx);
-- 
2.40.1

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

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