diff mbox series

[FFmpeg-devel,v2] libavcodec/pthread_frame: fix crash that call method ff_frame_thread_init failed because of mem insufficient

Message ID 20201016121303.68191-1-javashu2012@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] libavcodec/pthread_frame: fix crash that call method ff_frame_thread_init failed because of mem insufficient
Related show

Checks

Context Check Description
andriy/x86_make_warn warning New warnings during build
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make warning Make failed

Commit Message

徐慧书 Oct. 16, 2020, 12:13 p.m. UTC
From: bevis <javashu2012@gmail.com>

  Start planning to submit in two, This is my complete modification. 

Signed-off-by: bevis <javashu2012@gmail.com>
---
 libavcodec/pthread_frame.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Comments

Andreas Rheinhardt Oct. 19, 2020, 8:17 a.m. UTC | #1
javashu2012@gmail.com:
> From: bevis <javashu2012@gmail.com>
> 
>   Start planning to submit in two, This is my complete modification. 
> 
> Signed-off-by: bevis <javashu2012@gmail.com>
> ---
>  libavcodec/pthread_frame.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index f8a01ad8cd..3331fa0f6f 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -687,7 +687,7 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
>              pthread_join(p->thread, NULL);
>          p->thread_init=0;
>  
> -        if (codec->close && p->avctx)
> +        if (codec->close && p->avctx && p->avctx->priv_data)
>              codec->close(p->avctx);
>  
>          release_delayed_buffers(p);
> @@ -795,6 +795,11 @@ int ff_frame_thread_init(AVCodecContext *avctx)
>          pthread_cond_init(&p->progress_cond, NULL);
>          pthread_cond_init(&p->output_cond, NULL);
>  
> +        if (!copy) {
> +            err = AVERROR(ENOMEM);
> +            goto error;
> +        }
> +
>          p->frame = av_frame_alloc();
>          if (!p->frame) {
>              av_freep(&copy);
> @@ -802,22 +807,18 @@ int ff_frame_thread_init(AVCodecContext *avctx)
>              goto error;
>          }
>  
> -        p->parent = fctx;
> -        p->avctx  = copy;
> -
> -        if (!copy) {
> +        AVCodecInternal *internal = av_malloc(sizeof(AVCodecInternal));
> +        if (!internal) {
> +            av_freep(&copy);
>              err = AVERROR(ENOMEM);
>              goto error;
>          }
>  
> -        *copy = *src;
> +        p->parent = fctx;
> +        p->avctx  = copy;
>  
> -        copy->internal = av_malloc(sizeof(AVCodecInternal));
> -        if (!copy->internal) {
> -            copy->priv_data = NULL;
> -            err = AVERROR(ENOMEM);
> -            goto error;
> -        }
> +        *copy = *src;
> +        copy->internal = internal;
>          *copy->internal = *src->internal;
>          copy->internal->thread_ctx = p;
>          copy->internal->last_pkt_props = &p->avpkt;
> 
This will still call the close function even if av_opt_copy() failed or
if init failed; in the former case, you are not allowed to call it at
all and in the latter case you are only allowed to do so if the AVCodec
has the FF_CODEC_CAP_INIT_CLEANUP set.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index f8a01ad8cd..3331fa0f6f 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -687,7 +687,7 @@  void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
             pthread_join(p->thread, NULL);
         p->thread_init=0;
 
-        if (codec->close && p->avctx)
+        if (codec->close && p->avctx && p->avctx->priv_data)
             codec->close(p->avctx);
 
         release_delayed_buffers(p);
@@ -795,6 +795,11 @@  int ff_frame_thread_init(AVCodecContext *avctx)
         pthread_cond_init(&p->progress_cond, NULL);
         pthread_cond_init(&p->output_cond, NULL);
 
+        if (!copy) {
+            err = AVERROR(ENOMEM);
+            goto error;
+        }
+
         p->frame = av_frame_alloc();
         if (!p->frame) {
             av_freep(&copy);
@@ -802,22 +807,18 @@  int ff_frame_thread_init(AVCodecContext *avctx)
             goto error;
         }
 
-        p->parent = fctx;
-        p->avctx  = copy;
-
-        if (!copy) {
+        AVCodecInternal *internal = av_malloc(sizeof(AVCodecInternal));
+        if (!internal) {
+            av_freep(&copy);
             err = AVERROR(ENOMEM);
             goto error;
         }
 
-        *copy = *src;
+        p->parent = fctx;
+        p->avctx  = copy;
 
-        copy->internal = av_malloc(sizeof(AVCodecInternal));
-        if (!copy->internal) {
-            copy->priv_data = NULL;
-            err = AVERROR(ENOMEM);
-            goto error;
-        }
+        *copy = *src;
+        copy->internal = internal;
         *copy->internal = *src->internal;
         copy->internal->thread_ctx = p;
         copy->internal->last_pkt_props = &p->avpkt;