From patchwork Sun Jun 7 13:27:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 20187 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 196AD44BD71 for ; Sun, 7 Jun 2020 16:28:02 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EA3D068A646; Sun, 7 Jun 2020 16:28:01 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk1-f196.google.com (mail-qk1-f196.google.com [209.85.222.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 774CD6899EA for ; Sun, 7 Jun 2020 16:27:55 +0300 (EEST) Received: by mail-qk1-f196.google.com with SMTP id n11so14696314qkn.8 for ; Sun, 07 Jun 2020 06:27:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=cbRc4M7bp/wTlNUm+h1fySA1FjNrA7d/ieEzmQi9nGw=; b=ExrdTY3PAzU0evRjsAzIhQktMko2EEawR4XNArNhrwC7wCojhCM862GV4BSB4G6shf G9Xr7p4O9quJuLHT91aJi9wv77V59YZkhojILX11RNWo7+OxvLJDYT0PF9pMlGjpl6+S cUl6O7SmTr3mj1yyBpsUZ5CxLFd8RFFC0W7rACBNZCNywmq7swwGCPfr1oTWcXs8Aks4 YUan90WfiPYC7+GqYob240e4tLRAZeWnK6PjIZZMnpp/9GLSFuQ1Zh+cisKPa6kDyun7 med30AthZJMs0olAD0IXvHM8pCC9rW+B7dlslacwZ5Oe0KblaaJs5AslZJNTkP46gPV/ FgpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=cbRc4M7bp/wTlNUm+h1fySA1FjNrA7d/ieEzmQi9nGw=; b=K2Vpo5aRukCeqBx/ILAMAfB+CGfs7O11F6LUh67a+LxcVu9VYpjYXCjjaKLOmlisyX X+0fuO+h+YJ0IpJZUwdtxEdz+MzVJ5HbvBuiwzyo9MzxnN+p/NZ8objYpSJDbp+kXoQm 8iV42fSY1ehuya88zEYApQ3Mgi6ZPuncpKCXH6MDjIXkAC+PFlMLuGrKlAoHn6NP2oxf 0uYnEfgwo93R3wokGLqDHI4U2IoQ2dplLj6JqPSMS7xognkYMA2iQCz3qmgnhGrelAAJ qw/j4lDZmBI6iy+3dsQhMjF7CIX+Sa+ge93jl1CcdQOQx6qBnIql5JBGZuS8Qu3/tBHa RneA== X-Gm-Message-State: AOAM530FXUvBFAb72ov3Ou7PFyIuz+hET0jjRw2izIiA1oZnR6JdVfnJ /0/HqtiQfdbiQEQLucBFhpBEyGYS X-Google-Smtp-Source: ABdhPJwKRULZ5GkOGSuIzQHKLoq2rDIe0BTfX+wTBDHLaMGyg6nrPk80Bl+MEIPeLRTfwEUCMqTr+w== X-Received: by 2002:ae9:ef0e:: with SMTP id d14mr18395211qkg.416.1591536473236; Sun, 07 Jun 2020 06:27:53 -0700 (PDT) Received: from localhost.localdomain ([191.83.212.166]) by smtp.gmail.com with ESMTPSA id q207sm4554513qke.55.2020.06.07.06.27.52 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 07 Jun 2020 06:27:52 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Sun, 7 Jun 2020 10:27:37 -0300 Message-Id: <20200607132737.1375-1-jamrial@gmail.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] avcodec/cbs_h2645: keep separate parameter set lists for reading and writing X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Similar logic as 4e2bef6a82. In scearios where an Access Unit is written right after reading it using the same CBS context (hevc_metadata), the list of parsed parameters sets used by the writer must not be the one that's the result of the reader having already parsed the current Access Unit in question. Fixes: out of array access Fixes: 23034/clusterfuzz-testcase-minimized-ffmpeg_BSF_HEVC_METADATA_fuzzer-5074645169733632.fuzz Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: James Almer --- An alternative is forcing the usage of separate CBS contexts for reading and writing. libavcodec/cbs_h264.h | 18 ++++++++--- libavcodec/cbs_h2645.c | 72 ++++++++++++++++++++++++++++++++---------- libavcodec/cbs_h265.h | 26 +++++++++++---- 3 files changed, 89 insertions(+), 27 deletions(-) diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h index 9f7c2a0d30..9d104787d9 100644 --- a/libavcodec/cbs_h264.h +++ b/libavcodec/cbs_h264.h @@ -448,10 +448,20 @@ typedef struct CodedBitstreamH264Context { // All currently available parameter sets. These are updated when // any parameter set NAL unit is read/written with this context. - AVBufferRef *sps_ref[H264_MAX_SPS_COUNT]; - AVBufferRef *pps_ref[H264_MAX_PPS_COUNT]; - H264RawSPS *sps[H264_MAX_SPS_COUNT]; - H264RawPPS *pps[H264_MAX_PPS_COUNT]; + AVBufferRef **sps_ref; + AVBufferRef **pps_ref; + H264RawSPS **sps; + H264RawPPS **pps; + + AVBufferRef *read_sps_ref[H264_MAX_SPS_COUNT]; + AVBufferRef *read_pps_ref[H264_MAX_PPS_COUNT]; + H264RawSPS *read_sps[H264_MAX_SPS_COUNT]; + H264RawPPS *read_pps[H264_MAX_PPS_COUNT]; + + AVBufferRef *write_sps_ref[H264_MAX_SPS_COUNT]; + AVBufferRef *write_pps_ref[H264_MAX_PPS_COUNT]; + H264RawSPS *write_sps[H264_MAX_SPS_COUNT]; + H264RawPPS *write_pps[H264_MAX_PPS_COUNT]; // The currently active parameter sets. These are updated when any // NAL unit refers to the relevant parameter set. These pointers diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index b432921ecc..69ed890c63 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -758,14 +758,14 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, return 0; } -#define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \ +#define cbs_h2645_replace_ps(h26n, Hn, ps_name, ps_var, id_element) \ static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ CodedBitstreamUnit *unit) \ { \ CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \ H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \ unsigned int id = ps_var->id_element; \ - if (id >= FF_ARRAY_ELEMS(priv->ps_var)) { \ + if (id >= Hn ## _MAX_ ## ps_name ## _COUNT) { \ av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ " id : %d.\n", id); \ return AVERROR_INVALIDDATA; \ @@ -785,18 +785,24 @@ static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ return 0; \ } -cbs_h2645_replace_ps(4, SPS, sps, seq_parameter_set_id) -cbs_h2645_replace_ps(4, PPS, pps, pic_parameter_set_id) -cbs_h2645_replace_ps(5, VPS, vps, vps_video_parameter_set_id) -cbs_h2645_replace_ps(5, SPS, sps, sps_seq_parameter_set_id) -cbs_h2645_replace_ps(5, PPS, pps, pps_pic_parameter_set_id) +cbs_h2645_replace_ps(4, H264, SPS, sps, seq_parameter_set_id) +cbs_h2645_replace_ps(4, H264, PPS, pps, pic_parameter_set_id) +cbs_h2645_replace_ps(5, HEVC, VPS, vps, vps_video_parameter_set_id) +cbs_h2645_replace_ps(5, HEVC, SPS, sps, sps_seq_parameter_set_id) +cbs_h2645_replace_ps(5, HEVC, PPS, pps, pps_pic_parameter_set_id) static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit) { + CodedBitstreamH264Context *priv = ctx->priv_data; GetBitContext gbc; int err; + priv->sps_ref = (AVBufferRef **)priv->read_sps_ref; + priv->pps_ref = (AVBufferRef **)priv->read_pps_ref; + priv->sps = (H264RawSPS **)priv->read_sps; + priv->pps = (H264RawPPS **)priv->read_pps; + err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); if (err < 0) return err; @@ -953,9 +959,17 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit) { + CodedBitstreamH265Context *priv = ctx->priv_data; GetBitContext gbc; int err; + priv->vps_ref = (AVBufferRef **)priv->read_vps_ref; + priv->sps_ref = (AVBufferRef **)priv->read_sps_ref; + priv->pps_ref = (AVBufferRef **)priv->read_pps_ref; + priv->vps = (H265RawVPS **)priv->read_vps; + priv->sps = (H265RawSPS **)priv->read_sps; + priv->pps = (H265RawPPS **)priv->read_pps; + err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); if (err < 0) return err; @@ -1164,8 +1178,14 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit, PutBitContext *pbc) { + CodedBitstreamH264Context *priv = ctx->priv_data; int err; + priv->sps_ref = (AVBufferRef **)priv->write_sps_ref; + priv->pps_ref = (AVBufferRef **)priv->write_pps_ref; + priv->sps = (H264RawSPS **)priv->write_sps; + priv->pps = (H264RawPPS **)priv->write_pps; + switch (unit->type) { case H264_NAL_SPS: { @@ -1281,8 +1301,16 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, CodedBitstreamUnit *unit, PutBitContext *pbc) { + CodedBitstreamH265Context *priv = ctx->priv_data; int err; + priv->vps_ref = (AVBufferRef **)priv->write_vps_ref; + priv->sps_ref = (AVBufferRef **)priv->write_sps_ref; + priv->pps_ref = (AVBufferRef **)priv->write_pps_ref; + priv->vps = (H265RawVPS **)priv->write_vps; + priv->sps = (H265RawSPS **)priv->write_sps; + priv->pps = (H265RawPPS **)priv->write_pps; + switch (unit->type) { case HEVC_NAL_VPS: { @@ -1483,10 +1511,14 @@ static void cbs_h264_close(CodedBitstreamContext *ctx) ff_h2645_packet_uninit(&h264->common.read_packet); - for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++) - av_buffer_unref(&h264->sps_ref[i]); - for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++) - av_buffer_unref(&h264->pps_ref[i]); + for (i = 0; i < H264_MAX_SPS_COUNT; i++) { + av_buffer_unref(&h264->read_sps_ref[i]); + av_buffer_unref(&h264->write_sps_ref[i]); + } + for (i = 0; i < H264_MAX_PPS_COUNT; i++) { + av_buffer_unref(&h264->read_pps_ref[i]); + av_buffer_unref(&h264->write_pps_ref[i]); + } } static void cbs_h265_close(CodedBitstreamContext *ctx) @@ -1496,12 +1528,18 @@ static void cbs_h265_close(CodedBitstreamContext *ctx) ff_h2645_packet_uninit(&h265->common.read_packet); - for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++) - av_buffer_unref(&h265->vps_ref[i]); - for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++) - av_buffer_unref(&h265->sps_ref[i]); - for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++) - av_buffer_unref(&h265->pps_ref[i]); + for (i = 0; i < HEVC_MAX_VPS_COUNT; i++) { + av_buffer_unref(&h265->read_vps_ref[i]); + av_buffer_unref(&h265->write_vps_ref[i]); + } + for (i = 0; i < HEVC_MAX_SPS_COUNT; i++) { + av_buffer_unref(&h265->read_sps_ref[i]); + av_buffer_unref(&h265->write_sps_ref[i]); + } + for (i = 0; i < HEVC_MAX_PPS_COUNT; i++) { + av_buffer_unref(&h265->read_pps_ref[i]); + av_buffer_unref(&h265->write_pps_ref[i]); + } } const CodedBitstreamType ff_cbs_type_h264 = { diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h index 73897f77a4..ab27f77f15 100644 --- a/libavcodec/cbs_h265.h +++ b/libavcodec/cbs_h265.h @@ -731,12 +731,26 @@ typedef struct CodedBitstreamH265Context { // All currently available parameter sets. These are updated when // any parameter set NAL unit is read/written with this context. - AVBufferRef *vps_ref[HEVC_MAX_VPS_COUNT]; - AVBufferRef *sps_ref[HEVC_MAX_SPS_COUNT]; - AVBufferRef *pps_ref[HEVC_MAX_PPS_COUNT]; - H265RawVPS *vps[HEVC_MAX_VPS_COUNT]; - H265RawSPS *sps[HEVC_MAX_SPS_COUNT]; - H265RawPPS *pps[HEVC_MAX_PPS_COUNT]; + AVBufferRef **vps_ref; + AVBufferRef **sps_ref; + AVBufferRef **pps_ref; + H265RawVPS **vps; + H265RawSPS **sps; + H265RawPPS **pps; + + AVBufferRef *read_vps_ref[HEVC_MAX_VPS_COUNT]; + AVBufferRef *read_sps_ref[HEVC_MAX_SPS_COUNT]; + AVBufferRef *read_pps_ref[HEVC_MAX_PPS_COUNT]; + H265RawVPS *read_vps[HEVC_MAX_VPS_COUNT]; + H265RawSPS *read_sps[HEVC_MAX_SPS_COUNT]; + H265RawPPS *read_pps[HEVC_MAX_PPS_COUNT]; + + AVBufferRef *write_vps_ref[HEVC_MAX_VPS_COUNT]; + AVBufferRef *write_sps_ref[HEVC_MAX_SPS_COUNT]; + AVBufferRef *write_pps_ref[HEVC_MAX_PPS_COUNT]; + H265RawVPS *write_vps[HEVC_MAX_VPS_COUNT]; + H265RawSPS *write_sps[HEVC_MAX_SPS_COUNT]; + H265RawPPS *write_pps[HEVC_MAX_PPS_COUNT]; // The currently active parameter sets. These are updated when any // NAL unit refers to the relevant parameter set. These pointers