[FFmpeg-devel] [PATCH] hw_base_encode: Free pictures on close

[Cameron Gutman]

On Wed, Oct 16, 2024 at 5:49 AM Lynne via ffmpeg-devel
<ffmpeg-devel at ffmpeg.org> wrote:
>
> On 15/10/2024 17:51, Lynne via ffmpeg-devel wrote:
> > On 15/10/2024 16:49, David Rosca wrote:
> >> Fixes leaking recon surfaces with VAAPI.
> >> ---
> >>   libavcodec/hw_base_encode.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
> >> index 7b6ec97d3b..912c707a68 100644
> >> --- a/libavcodec/hw_base_encode.c
> >> +++ b/libavcodec/hw_base_encode.c
> >> @@ -804,6 +804,11 @@ int ff_hw_base_encode_init(AVCodecContext *avctx,
> >> FFHWBaseEncodeContext *ctx)
> >>   int ff_hw_base_encode_close(FFHWBaseEncodeContext *ctx)
> >>   {
> >> +    FFHWBaseEncodePicture *pic;
> >> +
> >> +    for (pic = ctx->pic_start; pic; pic = pic->next)
> >> +        base_encode_pic_free(pic);
> >> +
> >>       av_fifo_freep2(&ctx->encode_fifo);
> >>       av_frame_free(&ctx->frame);
> >
> > I've noticed this happening with Vulkan as well.
> >
> > LGTM, I'll push this after testing it in a few hours
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> Thanks, pushed

Aha, I was chasing a major VAAPI leak (VAAPI surfaces, DRI fds, etc)
in FFmpeg 7.1 when performing multiple VAAPI encoding sessions in a
single process that turned out to be fixed by this patch.
Unfortunately, this particular fix has a use-after-free since
base_encode_pic_free() frees pic before we read pic->next.

You probably want something like:
while (ctx->pic_start) {
    pic = ctx->pic_start->next;
    base_encode_pic_free(ctx->pic_start);
    ctx->pic_start = pic;
}

Since this is a 7.1 regression, I'd appreciate it if you could also
cherry-pick this commit and the UAF fix into release/7.1.