[FFmpeg-devel] [PATCH] fftools/ffmpeg_opt: Check before accessing union member

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

OptionDef.u is only an offset (i.e. its off member) iff OPT_FLAG_OFFSET
is true. Otherwise, the pointer arithmetic can be undefined behaviour.
UBSan warns about this (on 32bit arches):
src/fftools/ffmpeg_opt.c:102:15: runtime error: pointer index expression with base 0xffa4db10 overflowed to 0x56059a50

This commit fixes this by checking for OPT_FLAG_OFFSET first.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 fftools/ffmpeg_opt.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index a9a785a0ac..4763c555e8 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -94,13 +94,16 @@ int recast_media = 0;
 
 static void uninit_options(OptionsContext *o)
 {
-    const OptionDef *po = options;
     int i;
 
     /* all OPT_SPEC and OPT_TYPE_STRING can be freed in generic way */
-    while (po->name) {
-        void *dst = (uint8_t*)o + po->u.off;
+    for (const OptionDef *po = options; po->name; po++) {
+        void *dst;
 
+        if (!(po->flags & OPT_FLAG_OFFSET))
+            continue;
+
+        dst = (uint8_t*)o + po->u.off;
         if (po->flags & OPT_FLAG_SPEC) {
             SpecifierOptList *so = dst;
             for (int i = 0; i < so->nb_opt; i++) {
@@ -110,9 +113,8 @@ static void uninit_options(OptionsContext *o)
             }
             av_freep(&so->opt);
             so->nb_opt = 0;
-        } else if (po->flags & OPT_FLAG_OFFSET && po->type == OPT_TYPE_STRING)
+        } else if (po->type == OPT_TYPE_STRING)
             av_freep(dst);
-        po++;
     }
 
     for (i = 0; i < o->nb_stream_maps; i++)
-- 
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".