[FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: Fix the memory leak for enc_ctrl.Payload

Fu, Linjie linjie.fu at intel.com
Thu Apr 11 09:50:54 EEST 2019


> -----Original Message-----
> From: Li, Zhong
> Sent: Thursday, April 11, 2019 11:14
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Cc: Fu, Linjie <linjie.fu at intel.com>
> Subject: RE: [FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: Fix the memory leak
> for enc_ctrl.Payload
> 
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf
> > Of Linjie Fu
> > Sent: Wednesday, April 10, 2019 7:27 PM
> > To: ffmpeg-devel at ffmpeg.org
> > Cc: Fu, Linjie <linjie.fu at intel.com>
> > Subject: [FFmpeg-devel] [PATCH 1/2] lavc/qsvenc: Fix the memory leak for
> > enc_ctrl.Payload
> >
> > frame->enc_ctrl.Payload is malloced in get_free_frame, directly memset
> > the whole structure of enc_ctrl to zero will cause the memory leak for
> > enc_ctrl.Payload.
> >
> > Fix the memory leak issue and reset other members in mfxEncodeCtrl.
> >
> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >  libavcodec/qsvenc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c index
> > 5aa020d47b..029bb562d6 100644
> > --- a/libavcodec/qsvenc.c
> > +++ b/libavcodec/qsvenc.c
> > @@ -1254,7 +1254,7 @@ static int encode_frame(AVCodecContext *avctx,
> > QSVEncContext *q,
> >      if (qsv_frame) {
> >          surf = &qsv_frame->surface;
> >          enc_ctrl = &qsv_frame->enc_ctrl;
> > -        memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl));
> > +        memset(enc_ctrl, 0, sizeof(mfxEncodeCtrl) - sizeof(mfxPayload
> > + **));
> 
> Looks a little tricky, if someone didn't read carefully the sequence of
> mfxPayload usages, they can't know why this change happening.
> And potential risk for other elements such as "mfxExtBuffer** ExtParam" is
> still existed.
> 
> IMHO, one good habit is that is you want to memset a buffer, would better
> memset it when you allocate it.
> So I believe the correct fix should be: memset mfxEncodeCtrl at the start of
> qsv frame allocated: the place should be in the function get_free_frame()
> 
Aware of the risk of mfxExtBuffer too, and move the memset at the start of the allocation is great, will modify.

Thanks,
Linjie
 


More information about the ffmpeg-devel mailing list