diff mbox series

[FFmpeg-devel,1/4] avutil/detection_bbox: fix the memory leak on error

Message ID 1632836198-23044-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/4] avutil/detection_bbox: fix the memory leak on error
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Limin Wang Sept. 28, 2021, 1:36 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavutil/detection_bbox.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Rheinhardt Sept. 28, 2021, 1:45 p.m. UTC | #1
lance.lmwang@gmail.com:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavutil/detection_bbox.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
> index 40711e6..d066567 100644
> --- a/libavutil/detection_bbox.c
> +++ b/libavutil/detection_bbox.c
> @@ -61,6 +61,7 @@ AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32
>      }
>  
>      if (!av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_DETECTION_BBOXES, buf)) {
> +        av_freep(&header);
>          av_buffer_unref(&buf);
>          return NULL;
>      }
> 

This is wrong: header is owned by buf (or rather by the underlying
AVBuffer) and will automatically be freed when the last AVBufferRef gets
unreferenced, which happens in the above av_buffer_unref() call. Your
patch will just lead to a double-free.

- Andreas
Limin Wang Sept. 28, 2021, 1:58 p.m. UTC | #2
On Tue, Sep 28, 2021 at 03:45:03PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang@gmail.com:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavutil/detection_bbox.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
> > index 40711e6..d066567 100644
> > --- a/libavutil/detection_bbox.c
> > +++ b/libavutil/detection_bbox.c
> > @@ -61,6 +61,7 @@ AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32
> >      }
> >  
> >      if (!av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_DETECTION_BBOXES, buf)) {
> > +        av_freep(&header);
> >          av_buffer_unref(&buf);
> >          return NULL;
> >      }
> > 
> 
> This is wrong: header is owned by buf (or rather by the underlying
> AVBuffer) and will automatically be freed when the last AVBufferRef gets
> unreferenced, which happens in the above av_buffer_unref() call. Your
> patch will just lead to a double-free.

Sorry, it's my fault, haven't notice that, please ignore the patch.

> 
> - Andreas
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavutil/detection_bbox.c b/libavutil/detection_bbox.c
index 40711e6..d066567 100644
--- a/libavutil/detection_bbox.c
+++ b/libavutil/detection_bbox.c
@@ -61,6 +61,7 @@  AVDetectionBBoxHeader *av_detection_bbox_create_side_data(AVFrame *frame, uint32
     }
 
     if (!av_frame_new_side_data_from_buf(frame, AV_FRAME_DATA_DETECTION_BBOXES, buf)) {
+        av_freep(&header);
         av_buffer_unref(&buf);
         return NULL;
     }