From 700c00eda331f03b13cdee76ef51f89f1f6506c1 Mon Sep 17 00:00:00 2001 From: Seungha Yang Date: Sun, 21 Apr 2024 22:38:50 +0900 Subject: [PATCH] d3d12decoder: Fix potential use after free A DPB buffer held by codec picture object may not be writable at the moment, then gst_buffer_make_writable() will unref passed buffer. Specifically, the use after free or double free can happen if: * Crop meta of buffer copy is required because of non-zero top-left crop position * zero-copy is possible with crop meta * A picture was duplicated, interlaced h264 stream for example Interlaced h264 stream with non-zero top-left crop position is not very common but it's possible configuration in theory. Thus gst_buffer_make_writable() should be called with GstVideoCodecFrame.output_buffer directly. Part-of: --- .../sys/d3d12/gstd3d12decoder.cpp | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/subprojects/gst-plugins-bad/sys/d3d12/gstd3d12decoder.cpp b/subprojects/gst-plugins-bad/sys/d3d12/gstd3d12decoder.cpp index 644e2ca074..c283b39bd8 100644 --- a/subprojects/gst-plugins-bad/sys/d3d12/gstd3d12decoder.cpp +++ b/subprojects/gst-plugins-bad/sys/d3d12/gstd3d12decoder.cpp @@ -1360,6 +1360,7 @@ gst_d3d12_decoder_process_output (GstD3D12Decoder * self, ID3D12Resource *resource; UINT subresource[2]; HRESULT hr; + bool attach_crop_meta = false; auto priv = self->priv; @@ -1405,21 +1406,8 @@ gst_d3d12_decoder_process_output (GstD3D12Decoder * self, GST_MINI_OBJECT_FLAG_SET (dmem, GST_D3D12_MEMORY_TRANSFER_NEED_DOWNLOAD); GST_MINI_OBJECT_FLAG_UNSET (dmem, GST_D3D12_MEMORY_TRANSFER_NEED_UPLOAD); - if (priv->session->need_crop) { - GstVideoCropMeta *crop_meta; - - buffer = gst_buffer_make_writable (buffer); - crop_meta = gst_buffer_get_video_crop_meta (buffer); - if (!crop_meta) - crop_meta = gst_buffer_add_video_crop_meta (buffer); - - crop_meta->x = priv->session->crop_x; - crop_meta->y = priv->session->crop_y; - crop_meta->width = priv->session->info.width; - crop_meta->height = priv->session->info.height; - - GST_TRACE_OBJECT (self, "Attatching crop meta"); - } + if (priv->session->need_crop) + attach_crop_meta = true; frame->output_buffer = gst_buffer_ref (buffer); } else { @@ -1550,6 +1538,18 @@ gst_d3d12_decoder_process_output (GstD3D12Decoder * self, GST_BUFFER_FLAG_SET (frame->output_buffer, buffer_flags); gst_codec_picture_unref (picture); + if (attach_crop_meta) { + frame->output_buffer = gst_buffer_make_writable (frame->output_buffer); + + auto crop_meta = gst_buffer_add_video_crop_meta (frame->output_buffer); + crop_meta->x = priv->session->crop_x; + crop_meta->y = priv->session->crop_y; + crop_meta->width = priv->session->info.width; + crop_meta->height = priv->session->info.height; + + GST_TRACE_OBJECT (self, "Attatching crop meta"); + } + return gst_video_decoder_finish_frame (videodec, frame); error: