| Message ID | 20240624133133.11016-1-post@frankplowman.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavc/vvc: Validate IBC block vector | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, Jun 24, 2024 at 9:31 PM Frank Plowman <post@frankplowman.com> wrote: > From H.266 (V3) (09/2023) p. 321: > > It is a requirement of bitstream conformance that the luma block > vector bvL shall obey the following constraints: > - CtbSizeY is greater than or equal to > ((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight > > This patch checks this is true, which fixes crashes on fuzzed > bitstreams. > > Signed-off-by: Frank Plowman <post@frankplowman.com> > --- > libavcodec/vvc/intra.c | 25 ++++++++++++++++++++++--- > libavcodec/vvc/thread.c | 4 +--- > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c > index f77a012f09..11371db797 100644 > --- a/libavcodec/vvc/intra.c > +++ b/libavcodec/vvc/intra.c > @@ -624,15 +624,26 @@ static void intra_block_copy(const VVCLocalContext > *lc, const int c_idx) > } > } > > -static void vvc_predict_ibc(const VVCLocalContext *lc) > +static int vvc_predict_ibc(const VVCLocalContext *lc) > { > - const H266RawSPS *rsps = lc->fc->ps.sps->r; > + const VVCFrameContext *fc = lc->fc; > + const VVCSPS *sps = lc->fc->ps.sps; > + const H266RawSPS *rsps = sps->r; > + const CodingUnit *cu = lc->cu; > + const Mv *bv = &cu->pu.mi.mv[L0][0]; > + > + if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y - > 1)) + cu->cb_height) { > + av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple > CTBs.\n"); > + return AVERROR_INVALIDDATA; > + } Hi Frank, Thank you for the patch. Could we wrap it as a function and call it in ff_vvc_mvp_ibc and ff_vvc_luma_mv_merge_ibc? This will help us detect errors earlier. > > intra_block_copy(lc, LUMA); > if (lc->cu->tree_type == SINGLE_TREE && rsps->sps_chroma_format_idc) { > intra_block_copy(lc, CB); > intra_block_copy(lc, CR); > } > + > + return 0; > } > > static void ibc_fill_vir_buf(const VVCLocalContext *lc, const CodingUnit > *cu) > @@ -678,7 +689,10 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const int > rs, const int rx, const in > if (cu->ciip_flag) > ff_vvc_predict_ciip(lc); > else if (cu->pred_mode == MODE_IBC) > - vvc_predict_ibc(lc); > + ret = vvc_predict_ibc(lc); > + if (ret) > + goto fail; > + > if (cu->coded_flag) { > ret = reconstruct(lc); > } else { > @@ -687,10 +701,15 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const > int rs, const int rx, const in > if (sps->r->sps_chroma_format_idc && cu->tree_type != > DUAL_TREE_LUMA) > add_reconstructed_area(lc, CHROMA, cu->x0, cu->y0, > cu->cb_width, cu->cb_height); > } > + if (ret) > + goto fail; > + > if (sps->r->sps_ibc_enabled_flag) > ibc_fill_vir_buf(lc, cu); > cu = cu->next; > } > + > +fail: > ff_vvc_ctu_free_cus(ctu); > return ret; > } > diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c > index 8777d380bf..5b01dd2d20 100644 > --- a/libavcodec/vvc/thread.c > +++ b/libavcodec/vvc/thread.c > @@ -454,9 +454,7 @@ static int run_inter(VVCContext *s, VVCLocalContext > *lc, VVCTask *t) > > static int run_recon(VVCContext *s, VVCLocalContext *lc, VVCTask *t) > { > - ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry); > - > - return 0; > + return ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry); > } > > static int run_lmcs(VVCContext *s, VVCLocalContext *lc, VVCTask *t) > -- > 2.45.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 25/06/2024 15:19, Nuo Mi wrote: > On Mon, Jun 24, 2024 at 9:31 PM Frank Plowman <post@frankplowman.com> wrote: > >> From H.266 (V3) (09/2023) p. 321: >> >> It is a requirement of bitstream conformance that the luma block >> vector bvL shall obey the following constraints: >> - CtbSizeY is greater than or equal to >> ((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight >> >> This patch checks this is true, which fixes crashes on fuzzed >> bitstreams. >> >> Signed-off-by: Frank Plowman <post@frankplowman.com> >> --- >> libavcodec/vvc/intra.c | 25 ++++++++++++++++++++++--- >> libavcodec/vvc/thread.c | 4 +--- >> 2 files changed, 23 insertions(+), 6 deletions(-) >> >> diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c >> index f77a012f09..11371db797 100644 >> --- a/libavcodec/vvc/intra.c >> +++ b/libavcodec/vvc/intra.c >> @@ -624,15 +624,26 @@ static void intra_block_copy(const VVCLocalContext >> *lc, const int c_idx) >> } >> } >> >> -static void vvc_predict_ibc(const VVCLocalContext *lc) >> +static int vvc_predict_ibc(const VVCLocalContext *lc) >> { >> - const H266RawSPS *rsps = lc->fc->ps.sps->r; >> + const VVCFrameContext *fc = lc->fc; >> + const VVCSPS *sps = lc->fc->ps.sps; >> + const H266RawSPS *rsps = sps->r; >> + const CodingUnit *cu = lc->cu; >> + const Mv *bv = &cu->pu.mi.mv[L0][0]; >> + >> + if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y - >> 1)) + cu->cb_height) { >> + av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple >> CTBs.\n"); >> + return AVERROR_INVALIDDATA; >> + } > > Hi Frank, > Thank you for the patch. > Could we wrap it as a function and call it in ff_vvc_mvp_ibc and > ff_vvc_luma_mv_merge_ibc? > This will help us detect errors earlier. > I agree that's a better place to do the check. I didn't realise the code paths were already so separate for IBC there. v2 sent. Cheers, -- Frank
diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c index f77a012f09..11371db797 100644 --- a/libavcodec/vvc/intra.c +++ b/libavcodec/vvc/intra.c @@ -624,15 +624,26 @@ static void intra_block_copy(const VVCLocalContext *lc, const int c_idx) } } -static void vvc_predict_ibc(const VVCLocalContext *lc) +static int vvc_predict_ibc(const VVCLocalContext *lc) { - const H266RawSPS *rsps = lc->fc->ps.sps->r; + const VVCFrameContext *fc = lc->fc; + const VVCSPS *sps = lc->fc->ps.sps; + const H266RawSPS *rsps = sps->r; + const CodingUnit *cu = lc->cu; + const Mv *bv = &cu->pu.mi.mv[L0][0]; + + if (sps->ctb_size_y < ((cu->y0 + (bv->y >> 4)) & (sps->ctb_size_y - 1)) + cu->cb_height) { + av_log(fc->log_ctx, AV_LOG_ERROR, "IBC region spans multiple CTBs.\n"); + return AVERROR_INVALIDDATA; + } intra_block_copy(lc, LUMA); if (lc->cu->tree_type == SINGLE_TREE && rsps->sps_chroma_format_idc) { intra_block_copy(lc, CB); intra_block_copy(lc, CR); } + + return 0; } static void ibc_fill_vir_buf(const VVCLocalContext *lc, const CodingUnit *cu) @@ -678,7 +689,10 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const int rs, const int rx, const in if (cu->ciip_flag) ff_vvc_predict_ciip(lc); else if (cu->pred_mode == MODE_IBC) - vvc_predict_ibc(lc); + ret = vvc_predict_ibc(lc); + if (ret) + goto fail; + if (cu->coded_flag) { ret = reconstruct(lc); } else { @@ -687,10 +701,15 @@ int ff_vvc_reconstruct(VVCLocalContext *lc, const int rs, const int rx, const in if (sps->r->sps_chroma_format_idc && cu->tree_type != DUAL_TREE_LUMA) add_reconstructed_area(lc, CHROMA, cu->x0, cu->y0, cu->cb_width, cu->cb_height); } + if (ret) + goto fail; + if (sps->r->sps_ibc_enabled_flag) ibc_fill_vir_buf(lc, cu); cu = cu->next; } + +fail: ff_vvc_ctu_free_cus(ctu); return ret; } diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c index 8777d380bf..5b01dd2d20 100644 --- a/libavcodec/vvc/thread.c +++ b/libavcodec/vvc/thread.c @@ -454,9 +454,7 @@ static int run_inter(VVCContext *s, VVCLocalContext *lc, VVCTask *t) static int run_recon(VVCContext *s, VVCLocalContext *lc, VVCTask *t) { - ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry); - - return 0; + return ff_vvc_reconstruct(lc, t->rs, t->rx, t->ry); } static int run_lmcs(VVCContext *s, VVCLocalContext *lc, VVCTask *t)
From H.266 (V3) (09/2023) p. 321: It is a requirement of bitstream conformance that the luma block vector bvL shall obey the following constraints: - CtbSizeY is greater than or equal to ((yCb + (bvL[ 1 ] >> 4)) & (CtbSizeY − 1)) + cbHeight This patch checks this is true, which fixes crashes on fuzzed bitstreams. Signed-off-by: Frank Plowman <post@frankplowman.com> --- libavcodec/vvc/intra.c | 25 ++++++++++++++++++++++--- libavcodec/vvc/thread.c | 4 +--- 2 files changed, 23 insertions(+), 6 deletions(-)