[FFmpeg-devel] [PATCH] avcodec/cbs_h2645: keep separate parameter set lists for reading and writing

James Almer jamrial at gmail.com
Sun Jun 7 16:27:37 EEST 2020


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 <jamrial at gmail.com>
---
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
-- 
2.26.2



More information about the ffmpeg-devel mailing list