[FFmpeg-devel] [PATCH] avdevice/lavfi: Fix double-free on error

[FFmpeg-devel] [PATCH] avdevice/lavfi: Fix double-free on error

[Andreas Rheinhardt]

After the AVFrame has been wrapped into a buffer,
it is owned by the buffer and must not be freed manually
any more. Yet this happens on subsequent errors.

This bug was introduced in 6ca43a9675d651d7ea47c7ba2fafb1bf831c4d0b.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavdevice/lavfi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index ec7ebdbc90..2bfd0b81c7 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -365,7 +365,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
     LavfiContext *lavfi = avctx->priv_data;
     double min_pts = DBL_MAX;
     int stream_idx, min_pts_sink_idx = 0;
-    AVFrame *frame;
+    AVFrame *frame, *frame_to_free;
     AVDictionary *frame_metadata;
     int ret, i;
     AVStream *st;
@@ -378,6 +378,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
     frame = av_frame_alloc();
     if (!frame)
         return AVERROR(ENOMEM);
+    frame_to_free = frame;
 
     /* iterate through all the graph sinks. Select the sink with the
      * minimum PTS */
@@ -423,6 +424,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
             ret = AVERROR(ENOMEM);
             goto fail;
         }
+        frame_to_free = NULL;
 
         pkt->data   = pkt->buf->data;
         pkt->size   = pkt->buf->size;
@@ -463,12 +465,11 @@ FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-    if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO)
-        av_frame_free(&frame);
+    av_frame_free(&frame_to_free);
 
     return pkt->size;
 fail:
-    av_frame_free(&frame);
+    av_frame_free(&frame_to_free);
     return ret;
 
 }
-- 
2.34.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".

Re: [FFmpeg-devel] [PATCH] avdevice/lavfi: Fix double-free on error

[Timo Rothenpieler]

On 29.09.2023 19:28, Andreas Rheinhardt wrote:
> After the AVFrame has been wrapped into a buffer,
> it is owned by the buffer and must not be freed manually
> any more. Yet this happens on subsequent errors.
> 
> This bug was introduced in 6ca43a9675d651d7ea47c7ba2fafb1bf831c4d0b.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavdevice/lavfi.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> index ec7ebdbc90..2bfd0b81c7 100644
> --- a/libavdevice/lavfi.c
> +++ b/libavdevice/lavfi.c
> @@ -365,7 +365,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>       LavfiContext *lavfi = avctx->priv_data;
>       double min_pts = DBL_MAX;
>       int stream_idx, min_pts_sink_idx = 0;
> -    AVFrame *frame;
> +    AVFrame *frame, *frame_to_free;
>       AVDictionary *frame_metadata;
>       int ret, i;
>       AVStream *st;
> @@ -378,6 +378,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>       frame = av_frame_alloc();
>       if (!frame)
>           return AVERROR(ENOMEM);
> +    frame_to_free = frame;
>   
>       /* iterate through all the graph sinks. Select the sink with the
>        * minimum PTS */
> @@ -423,6 +424,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>               ret = AVERROR(ENOMEM);
>               goto fail;
>           }
> +        frame_to_free = NULL;
>   
>           pkt->data   = pkt->buf->data;
>           pkt->size   = pkt->buf->size;
> @@ -463,12 +465,11 @@ FF_DISABLE_DEPRECATION_WARNINGS
>   FF_ENABLE_DEPRECATION_WARNINGS
>   #endif
>   
> -    if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO)
> -        av_frame_free(&frame);
> +    av_frame_free(&frame_to_free);
>   
>       return pkt->size;
>   fail:
> -    av_frame_free(&frame);
> +    av_frame_free(&frame_to_free);
>       return ret;
>   
>   }

Looks sensible to me
_______________________________________________
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".