Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
@ 2025-02-21  3:31 Nil Fons Miret via ffmpeg-devel
  2025-03-03 17:07 ` Nil Fons Miret via ffmpeg-devel
  2025-03-03 23:56 ` Michael Niedermayer
  0 siblings, 2 replies; 5+ messages in thread
From: Nil Fons Miret via ffmpeg-devel @ 2025-02-21  3:31 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Nil Fons Miret

[-- Attachment #1: Type: text/plain, Size: 399 bytes --]

The return value of ff_draw_init and ff_draw_init2 are not checked in
most usages. However, if they return an error, they don't get to the
point where they set the attributes of the FFDrawContext. These
functions are typically used in conjunction with ff_draw_color, which
checks draw->desc->flags, causing a null pointer dereference.

Attaching patch with a fix to guard against this.

Thanks,
Nil

[-- Attachment #2: 0001-libavfilter-guard-against-ff_draw_init-ff_draw_init2-f.eml --]
[-- Type: message/rfc822, Size: 15830 bytes --]

From: Nil Fons Miret <nilf@netflix.com>
To: ffmpeg-devel@ffmpeg.org
Subject: [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
Date: Fri, 21 Feb 2025 01:18:21 +0000

The return value of ff_draw_init and ff_draw_init2 are not checked in
most usages. However, if they return an error, they don't get to the
point where they set the attributes of the FFDrawContext. These
functions are typically used in conjunction with ff_draw_color, which
checks draw->desc->flags, causing a null pointer dereference.

Signed-off-by: Nil Fons Miret <nilf@netflix.com>
---
 libavfilter/qrencode.c       | 26 +++++++++++++++++++++-----
 libavfilter/src_avsynctest.c |  7 ++++++-
 libavfilter/vf_datascope.c   | 31 +++++++++++++++++++++++++------
 libavfilter/vf_drawtext.c    |  6 +++++-
 libavfilter/vf_pad.c         |  6 +++++-
 libavfilter/vf_shear.c       |  7 ++++++-
 libavfilter/vf_stack.c       |  6 +++++-
 libavfilter/vf_subtitles.c   | 16 +++++++++++-----
 libavfilter/vf_tile.c        |  7 ++++++-
 libavfilter/vf_tinterlace.c  |  6 +++++-
 libavfilter/vf_tpad.c        |  7 ++++++-
 libavfilter/vsrc_testsrc.c   |  9 +++++++--
 12 files changed, 108 insertions(+), 26 deletions(-)

diff --git a/libavfilter/qrencode.c b/libavfilter/qrencode.c
index f96cc8dc93..0eb4dd5c5c 100644
--- a/libavfilter/qrencode.c
+++ b/libavfilter/qrencode.c
@@ -636,11 +636,19 @@ static int qrencodesrc_config_props(AVFilterLink *outlink)
         return AVERROR(EINVAL);
     }
 
-    ff_draw_init(&qr->draw, AV_PIX_FMT_ARGB, FF_DRAW_PROCESS_ALPHA);
+    ret = ff_draw_init(&qr->draw, AV_PIX_FMT_ARGB, FF_DRAW_PROCESS_ALPHA);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&qr->draw, &qr->draw_foreground_color, (const uint8_t *)&qr->foreground_color);
     ff_draw_color(&qr->draw, &qr->draw_background_color, (const uint8_t *)&qr->background_color);
 
-    ff_draw_init2(&qr->draw0, outlink->format, outlink->colorspace, outlink->color_range, FF_DRAW_PROCESS_ALPHA);
+    ret = ff_draw_init2(&qr->draw0, outlink->format, outlink->colorspace, outlink->color_range, FF_DRAW_PROCESS_ALPHA);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&qr->draw0, &qr->draw0_background_color, (const uint8_t *)&qr->background_color);
 
     outlink->w = qr->rendered_padded_qrcode_width;
@@ -734,8 +742,12 @@ static int qrencode_config_input(AVFilterLink *inlink)
 
     qr->is_source = 0;
 
-    ff_draw_init2(&qr->draw, inlink->format, inlink->colorspace, inlink->color_range,
-                  FF_DRAW_PROCESS_ALPHA);
+    ret = ff_draw_init2(&qr->draw, inlink->format, inlink->colorspace, inlink->color_range,
+                        FF_DRAW_PROCESS_ALPHA);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
 
     V(W) = V(main_w) = inlink->w;
     V(H) = V(main_h) = inlink->h;
@@ -764,8 +776,12 @@ static int qrencode_config_input(AVFilterLink *inlink)
     PARSE_EXPR(rendered_qrcode_width);
     PARSE_EXPR(rendered_padded_qrcode_width);
 
-    ff_draw_init2(&qr->draw, inlink->format, inlink->colorspace, inlink->color_range,
+    ret = ff_draw_init2(&qr->draw, inlink->format, inlink->colorspace, inlink->color_range,
                   FF_DRAW_PROCESS_ALPHA);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&qr->draw, &qr->draw_foreground_color, (const uint8_t *)&qr->foreground_color);
     ff_draw_color(&qr->draw, &qr->draw_background_color, (const uint8_t *)&qr->background_color);
 
diff --git a/libavfilter/src_avsynctest.c b/libavfilter/src_avsynctest.c
index 6f42137f49..68dffba43a 100644
--- a/libavfilter/src_avsynctest.c
+++ b/libavfilter/src_avsynctest.c
@@ -147,6 +147,7 @@ static av_cold int config_props(AVFilterLink *outlink)
     FilterLink *l = ff_filter_link(outlink);
     AVFilterContext *ctx = outlink->src;
     AVSyncTestContext *s = ctx->priv;
+    int ret;
 
     outlink->w = s->w;
     outlink->h = s->h;
@@ -160,7 +161,11 @@ static av_cold int config_props(AVFilterLink *outlink)
     s->dir = 1;
     s->prev_intpart = INT64_MIN;
 
-    ff_draw_init2(&s->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+    ret = ff_draw_init2(&s->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
 
     ff_draw_color(&s->draw, &s->fg, s->rgba[0]);
     ff_draw_color(&s->draw, &s->bg, s->rgba[1]);
diff --git a/libavfilter/vf_datascope.c b/libavfilter/vf_datascope.c
index 75dc149fbd..6efeb875a6 100644
--- a/libavfilter/vf_datascope.c
+++ b/libavfilter/vf_datascope.c
@@ -382,11 +382,18 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
 static int config_input(AVFilterLink *inlink)
 {
-    DatascopeContext *s = inlink->dst->priv;
+    AVFilterContext *ctx = inlink->dst;
+    DatascopeContext *s = ctx->priv;
+
     uint8_t alpha = s->opacity * 255;
+    int ret;
 
     s->nb_planes = av_pix_fmt_count_planes(inlink->format);
-    ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    ret = ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->draw, &s->white,  (uint8_t[]){ 255, 255, 255, 255} );
     ff_draw_color(&s->draw, &s->black,  (uint8_t[]){ 0, 0, 0, alpha} );
     ff_draw_color(&s->draw, &s->yellow, (uint8_t[]){ 255, 255, 0, 255} );
@@ -509,10 +516,16 @@ AVFILTER_DEFINE_CLASS(pixscope);
 
 static int pixscope_config_input(AVFilterLink *inlink)
 {
-    PixscopeContext *s = inlink->dst->priv;
+    AVFilterContext *ctx = inlink->dst;
+    PixscopeContext *s = ctx->priv;
+    int ret;
 
     s->nb_planes = av_pix_fmt_count_planes(inlink->format);
-    ff_draw_init(&s->draw, inlink->format, 0);
+    ret = ff_draw_init(&s->draw, inlink->format, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->draw, &s->dark,  (uint8_t[]){ 0, 0, 0, s->o * 255} );
     ff_draw_color(&s->draw, &s->black, (uint8_t[]){ 0, 0, 0, 255} );
     ff_draw_color(&s->draw, &s->white, (uint8_t[]){ 255, 255, 255, 255} );
@@ -927,11 +940,17 @@ static void update_oscilloscope(AVFilterContext *ctx)
 
 static int oscilloscope_config_input(AVFilterLink *inlink)
 {
-    OscilloscopeContext *s = inlink->dst->priv;
+    AVFilterContext *ctx = inlink->dst;
+    OscilloscopeContext *s = ctx->priv;
     int size;
+    int ret;
 
     s->nb_planes = av_pix_fmt_count_planes(inlink->format);
-    ff_draw_init(&s->draw, inlink->format, 0);
+    ret = ff_draw_init(&s->draw, inlink->format, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->draw, &s->black,   (uint8_t[]){   0,   0,   0, 255} );
     ff_draw_color(&s->draw, &s->white,   (uint8_t[]){ 255, 255, 255, 255} );
     ff_draw_color(&s->draw, &s->green,   (uint8_t[]){   0, 255,   0, 255} );
diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
index e4662f3a45..9f5d8d9998 100644
--- a/libavfilter/vf_drawtext.c
+++ b/libavfilter/vf_drawtext.c
@@ -1156,7 +1156,11 @@ static int config_input(AVFilterLink *inlink)
     char *expr;
     int ret;
 
-    ff_draw_init2(&s->dc, inlink->format, inlink->colorspace, inlink->color_range, FF_DRAW_PROCESS_ALPHA);
+    ret = ff_draw_init2(&s->dc, inlink->format, inlink->colorspace, inlink->color_range, FF_DRAW_PROCESS_ALPHA);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->dc, &s->fontcolor,   s->fontcolor.rgba);
     ff_draw_color(&s->dc, &s->shadowcolor, s->shadowcolor.rgba);
     ff_draw_color(&s->dc, &s->bordercolor, s->bordercolor.rgba);
diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
index 49fb272b24..0c7a1d2b44 100644
--- a/libavfilter/vf_pad.c
+++ b/libavfilter/vf_pad.c
@@ -114,7 +114,11 @@ static int config_input(AVFilterLink *inlink)
     double var_values[VARS_NB], res;
     char *expr;
 
-    ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    ret = ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->draw, &s->color, s->rgba_color);
 
     var_values[VAR_IN_W]  = var_values[VAR_IW] = inlink->w;
diff --git a/libavfilter/vf_shear.c b/libavfilter/vf_shear.c
index f76ed1e4d5..2e558c0044 100644
--- a/libavfilter/vf_shear.c
+++ b/libavfilter/vf_shear.c
@@ -250,6 +250,7 @@ static int config_output(AVFilterLink *outlink)
     AVFilterContext *ctx = outlink->src;
     ShearContext *s = ctx->priv;
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(outlink->format);
+    int ret;
 
     s->nb_planes = av_pix_fmt_count_planes(outlink->format);
     s->depth = desc->comp[0].depth;
@@ -260,7 +261,11 @@ static int config_output(AVFilterLink *outlink)
     s->planeheight[1] = s->planeheight[2] = AV_CEIL_RSHIFT(ctx->inputs[0]->h, desc->log2_chroma_h);
     s->planeheight[0] = s->planeheight[3] = ctx->inputs[0]->h;
 
-    ff_draw_init2(&s->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+    ret = ff_draw_init2(&s->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&s->draw, &s->color, s->fillcolor);
 
     s->filter_slice[0] = s->depth <= 8 ? filter_slice_nn8 : filter_slice_nn16;
diff --git a/libavfilter/vf_stack.c b/libavfilter/vf_stack.c
index 937b87305f..fa202ee0cc 100644
--- a/libavfilter/vf_stack.c
+++ b/libavfilter/vf_stack.c
@@ -312,7 +312,11 @@ static int config_output(AVFilterLink *outlink)
 
         if (s->fillcolor_enable) {
             const AVFilterLink *inlink = ctx->inputs[0];
-            ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+            ret = ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+            if (ret < 0) {
+                av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+                return ret;
+            }
             ff_draw_color(&s->draw, &s->color, s->fillcolor);
         }
 
diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
index 9f573ec4ed..cf23b7822a 100644
--- a/libavfilter/vf_subtitles.c
+++ b/libavfilter/vf_subtitles.c
@@ -182,12 +182,18 @@ static int query_formats(const AVFilterContext *ctx,
 
 static int config_input(AVFilterLink *inlink)
 {
-    AssContext *ass = inlink->dst->priv;
+    AVFilterContext *ctx = inlink->dst;
+    AssContext *ass = ctx->priv;
+    int ret;
 
-    ff_draw_init2(&ass->draw, inlink->format,
-                  ass_get_color_space(ass->track->YCbCrMatrix, inlink->colorspace),
-                  ass_get_color_range(ass->track->YCbCrMatrix, inlink->color_range),
-                  ass->alpha ? FF_DRAW_PROCESS_ALPHA : 0);
+    ret = ff_draw_init2(&ass->draw, inlink->format,
+                        ass_get_color_space(ass->track->YCbCrMatrix, inlink->colorspace),
+                        ass_get_color_range(ass->track->YCbCrMatrix, inlink->color_range),
+                        ass->alpha ? FF_DRAW_PROCESS_ALPHA : 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
 
     ass_set_frame_size  (ass->renderer, inlink->w, inlink->h);
     if (ass->original_w && ass->original_h) {
diff --git a/libavfilter/vf_tile.c b/libavfilter/vf_tile.c
index 9305123b33..7f2f212c4a 100644
--- a/libavfilter/vf_tile.c
+++ b/libavfilter/vf_tile.c
@@ -128,6 +128,7 @@ static int config_props(AVFilterLink *outlink)
     FilterLink *ol = ff_filter_link(outlink);
     const unsigned total_margin_w = (tile->w - 1) * tile->padding + 2*tile->margin;
     const unsigned total_margin_h = (tile->h - 1) * tile->padding + 2*tile->margin;
+    int ret;
 
     if (inlink->w > (INT_MAX - total_margin_w) / tile->w) {
         av_log(ctx, AV_LOG_ERROR, "Total width %ux%u is too much.\n",
@@ -143,7 +144,11 @@ static int config_props(AVFilterLink *outlink)
     outlink->h = tile->h * inlink->h + total_margin_h;
     outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
     ol->frame_rate = av_mul_q(il->frame_rate, av_make_q(1, tile->nb_frames - tile->overlap));
-    ff_draw_init2(&tile->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    ret = ff_draw_init2(&tile->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
     ff_draw_color(&tile->draw, &tile->blank, tile->rgba_color);
 
     return 0;
diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
index de7cd88103..f013891f3e 100644
--- a/libavfilter/vf_tinterlace.c
+++ b/libavfilter/vf_tinterlace.c
@@ -228,7 +228,11 @@ static int config_out_props(AVFilterLink *outlink)
 
     if (tinterlace->mode == MODE_PAD) {
         uint8_t black[4] = { 0, 0, 0, 16 };
-        ff_draw_init2(&tinterlace->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+        ret = ff_draw_init2(&tinterlace->draw, outlink->format, outlink->colorspace, outlink->color_range, 0);
+        if (ret < 0) {
+            av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+            return ret;
+        }
         ff_draw_color(&tinterlace->draw, &tinterlace->color, black);
         /* limited range */
         if (!ff_fmt_is_in(outlink->format, full_scale_yuvj_pix_fmts)) {
diff --git a/libavfilter/vf_tpad.c b/libavfilter/vf_tpad.c
index 7ba6fe72bf..f498bf4a3e 100644
--- a/libavfilter/vf_tpad.c
+++ b/libavfilter/vf_tpad.c
@@ -206,9 +206,14 @@ static int config_input(AVFilterLink *inlink)
     AVFilterContext *ctx = inlink->dst;
     FilterLink *l = ff_filter_link(inlink);
     TPadContext *s = ctx->priv;
+    int ret;
 
     if (needs_drawing(s)) {
-        ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+        ret = ff_draw_init2(&s->draw, inlink->format, inlink->colorspace, inlink->color_range, 0);
+        if (ret < 0) {
+            av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+            return ret;
+        }
         ff_draw_color(&s->draw, &s->color, s->rgba_color);
     }
 
diff --git a/libavfilter/vsrc_testsrc.c b/libavfilter/vsrc_testsrc.c
index c842cacceb..6be274cc90 100644
--- a/libavfilter/vsrc_testsrc.c
+++ b/libavfilter/vsrc_testsrc.c
@@ -262,8 +262,13 @@ static int color_config_props(AVFilterLink *inlink)
     TestSourceContext *test = ctx->priv;
     int ret;
 
-    ff_draw_init2(&test->draw, inlink->format, inlink->colorspace,
-                  inlink->color_range, 0);
+    ret = ff_draw_init2(&test->draw, inlink->format, inlink->colorspace,
+                        inlink->color_range, 0);
+    if (ret < 0) {
+        av_log(ctx, AV_LOG_ERROR, "Failed to initialize FFDrawContext\n");
+        return ret;
+    }
+
     ff_draw_color(&test->draw, &test->color, test->color_rgba);
 
     if (av_image_check_size(test->w, test->h, 0, ctx) < 0)
-- 
2.47.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
  2025-02-21  3:31 [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures Nil Fons Miret via ffmpeg-devel
@ 2025-03-03 17:07 ` Nil Fons Miret via ffmpeg-devel
  2025-03-03 23:56 ` Michael Niedermayer
  1 sibling, 0 replies; 5+ messages in thread
From: Nil Fons Miret via ffmpeg-devel @ 2025-03-03 17:07 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Nil Fons Miret

Hi, any feedback on this? Thank you

On Thu, Feb 20, 2025 at 7:31 PM Nil Fons Miret <nilf@netflix.com> wrote:
>
> The return value of ff_draw_init and ff_draw_init2 are not checked in
> most usages. However, if they return an error, they don't get to the
> point where they set the attributes of the FFDrawContext. These
> functions are typically used in conjunction with ff_draw_color, which
> checks draw->desc->flags, causing a null pointer dereference.
>
> Attaching patch with a fix to guard against this.
>
> Thanks,
> Nil
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
  2025-02-21  3:31 [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures Nil Fons Miret via ffmpeg-devel
  2025-03-03 17:07 ` Nil Fons Miret via ffmpeg-devel
@ 2025-03-03 23:56 ` Michael Niedermayer
  2025-03-04  1:05   ` Nil Fons Miret via ffmpeg-devel
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Niedermayer @ 2025-03-03 23:56 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 864 bytes --]

On Thu, Feb 20, 2025 at 10:31:57PM -0500, Nil Fons Miret via ffmpeg-devel wrote:
> The return value of ff_draw_init and ff_draw_init2 are not checked in
> most usages. However, if they return an error, they don't get to the
> point where they set the attributes of the FFDrawContext. These
> functions are typically used in conjunction with ff_draw_color, which
> checks draw->desc->flags, causing a null pointer dereference.
> 
> Attaching patch with a fix to guard against this.

Is every of these ff_draw_init* calls able to fail ?
i see one is called with constants for example

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
  2025-03-03 23:56 ` Michael Niedermayer
@ 2025-03-04  1:05   ` Nil Fons Miret via ffmpeg-devel
  2025-03-04  1:09     ` Michael Niedermayer
  0 siblings, 1 reply; 5+ messages in thread
From: Nil Fons Miret via ffmpeg-devel @ 2025-03-04  1:05 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Nil Fons Miret

From what I can see, that one time it is called with constants in
qrencode.c will never fail. I can remove that check, but I am a bit
worried it will make it brittle if the internals of ff_draw_init*
change later on, e.g. to introduce more failure modes. In fact, I ran
across this issue while upgrading an old ffmpeg that did not have some
of these checks, so it was one of the new checks that caused the
segfault. Let me know what you think, I am open to changing it.

Thank you,
Nil


On Mon, Mar 3, 2025 at 3:56 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Feb 20, 2025 at 10:31:57PM -0500, Nil Fons Miret via ffmpeg-devel wrote:
> > The return value of ff_draw_init and ff_draw_init2 are not checked in
> > most usages. However, if they return an error, they don't get to the
> > point where they set the attributes of the FFDrawContext. These
> > functions are typically used in conjunction with ff_draw_color, which
> > checks draw->desc->flags, causing a null pointer dereference.
> >
> > Attaching patch with a fix to guard against this.
>
> Is every of these ff_draw_init* calls able to fail ?
> i see one is called with constants for example
>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Into a blind darkness they enter who follow after the Ignorance,
> they as if into a greater darkness enter who devote themselves
> to the Knowledge alone. -- Isha Upanishad
> _______________________________________________
> 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".
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures
  2025-03-04  1:05   ` Nil Fons Miret via ffmpeg-devel
@ 2025-03-04  1:09     ` Michael Niedermayer
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Niedermayer @ 2025-03-04  1:09 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 934 bytes --]

On Mon, Mar 03, 2025 at 05:05:58PM -0800, Nil Fons Miret via ffmpeg-devel wrote:
> From what I can see, that one time it is called with constants in
> qrencode.c will never fail. I can remove that check, but I am a bit
> worried it will make it brittle if the internals of ff_draw_init*
> change later on, e.g. to introduce more failure modes. In fact, I ran
> across this issue while upgrading an old ffmpeg that did not have some
> of these checks, so it was one of the new checks that caused the
> segfault. Let me know what you think, I am open to changing it.

I think the important bit, is it should be documented.
Its not so important if theres a check. Even with a check it
could be documented that the case cannot occur currently

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-03-04  1:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-21  3:31 [FFmpeg-devel] [PATCH] libavfilter: guard against ff_draw_init/ff_draw_init2 failures Nil Fons Miret via ffmpeg-devel
2025-03-03 17:07 ` Nil Fons Miret via ffmpeg-devel
2025-03-03 23:56 ` Michael Niedermayer
2025-03-04  1:05   ` Nil Fons Miret via ffmpeg-devel
2025-03-04  1:09     ` Michael Niedermayer

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git