* [FFmpeg-devel] [PATCH v2 1/2] lavc/vvc: Don't discard palette_subblock_data return code @ 2025-07-15 18:48 Frank Plowman 2025-07-15 18:48 ` [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Add max parameter to kth_order_egk_decode Frank Plowman 0 siblings, 1 reply; 3+ messages in thread From: Frank Plowman @ 2025-07-15 18:48 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Frank Plowman, nuomi2021, toqsxw Signed-off-by: Frank Plowman <post@frankplowman.com> --- libavcodec/vvc/ctu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c index cf7edccb8b..35c18e78f6 100644 --- a/libavcodec/vvc/ctu.c +++ b/libavcodec/vvc/ctu.c @@ -2118,9 +2118,12 @@ static int hls_palette_coding(VVCLocalContext *lc, const VVCTreeType tree_type) palette_qp(lc, tree_type, escape_present); index[0] = 0; - for (int i = 0; i <= (cu->cb_width * cu->cb_height - 1) >> 4; i++) - palette_subblock_data(lc, max_index, i, transpose, + for (int i = 0; i <= (cu->cb_width * cu->cb_height - 1) >> 4; i++) { + ret = palette_subblock_data(lc, max_index, i, transpose, run_type, index, &prev_run_pos, &adjust); + if (ret < 0) + return ret; + } return 0; } -- 2.47.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 3+ messages in thread
* [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Add max parameter to kth_order_egk_decode 2025-07-15 18:48 [FFmpeg-devel] [PATCH v2 1/2] lavc/vvc: Don't discard palette_subblock_data return code Frank Plowman @ 2025-07-15 18:48 ` Frank Plowman 2025-07-16 10:08 ` Frank Plowman 0 siblings, 1 reply; 3+ messages in thread From: Frank Plowman @ 2025-07-15 18:48 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Frank Plowman, nuomi2021, toqsxw Prior to this patch, kth_order_egk_decode could read arbitrarily large values which then overflowed and caused various issues. Patch fixes this by making kth_order_egk_decode falliable, requiring the caller to specify an upper bound and returning an error if the read value would exceed that bound. This patch resolves the same issue as eb52251c0ab025b6b40b28994bc9dc616813b190, but I think this is the proper fix as it also addresses issues with syntax elements besides ff_vvc_num_signalled_palette_entries. Patch also includes a minor fix in hls_palette_coding, where the error code returned by palette_subblock_data was previously unchecked. Signed-off-by: Frank Plowman <post@frankplowman.com> --- Changes since v1: * Split change to hls_palette_coding to its own commit. * Return values from syntax functions in return val, rather than by modifying pointer parameter. --- libavcodec/vvc/cabac.c | 19 ++++++++++++------- libavcodec/vvc/cabac.h | 6 +++--- libavcodec/vvc/ctu.c | 30 ++++++++++++++++++------------ 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c index 6847ce59af..c2dbd46709 100644 --- a/libavcodec/vvc/cabac.c +++ b/libavcodec/vvc/cabac.c @@ -929,7 +929,7 @@ static int truncated_binary_decode(VVCLocalContext *lc, const int c_max) } // 9.3.3.5 k-th order Exp - Golomb binarization process -static int kth_order_egk_decode(CABACContext *c, int k) +static int kth_order_egk_decode(CABACContext *c, int k, const int max) { int bit = 1; int value = 0; @@ -937,6 +937,8 @@ static int kth_order_egk_decode(CABACContext *c, int k) while (bit) { bit = get_cabac_bypass(c); + if (max - value < (bit << k)) + return AVERROR_INVALIDDATA; value += bit << k++; } @@ -946,6 +948,9 @@ static int kth_order_egk_decode(CABACContext *c, int k) value += symbol; } + if (value > max) + return AVERROR_INVALIDDATA; + return value; } @@ -1377,14 +1382,14 @@ int ff_vvc_intra_chroma_pred_mode(VVCLocalContext *lc) return (get_cabac_bypass(&lc->ep->cc) << 1) | get_cabac_bypass(&lc->ep->cc); } -int ff_vvc_palette_predictor_run(VVCLocalContext *lc) +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max) { - return kth_order_egk_decode(&lc->ep->cc, 0); + return kth_order_egk_decode(&lc->ep->cc, 0, max); } -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc) +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int max) { - return kth_order_egk_decode(&lc->ep->cc, 0); + return kth_order_egk_decode(&lc->ep->cc, 0, max); } int ff_vvc_new_palette_entries(VVCLocalContext *lc, const int bit_depth) @@ -1424,9 +1429,9 @@ int ff_vvc_palette_idx_idc(VVCLocalContext *lc, const int max_palette_index, con return truncated_binary_decode(lc, max_palette_index - adjust); } -int ff_vvc_palette_escape_val(VVCLocalContext *lc) +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max) { - return kth_order_egk_decode(&lc->ep->cc, 5); + return kth_order_egk_decode(&lc->ep->cc, 5, max); } int ff_vvc_general_merge_flag(VVCLocalContext *lc) diff --git a/libavcodec/vvc/cabac.h b/libavcodec/vvc/cabac.h index 972890317e..6a0e713d19 100644 --- a/libavcodec/vvc/cabac.h +++ b/libavcodec/vvc/cabac.h @@ -81,15 +81,15 @@ int ff_vvc_intra_luma_mpm_remainder(VVCLocalContext *lc); int ff_vvc_cclm_mode_flag(VVCLocalContext *lc); int ff_vvc_cclm_mode_idx(VVCLocalContext *lc); int ff_vvc_intra_chroma_pred_mode(VVCLocalContext *lc); -int ff_vvc_palette_predictor_run(VVCLocalContext *lc); -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc); +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max); +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int max); int ff_vvc_new_palette_entries(VVCLocalContext *lc, int bit_dpeth); bool ff_vvc_palette_escape_val_present_flag(VVCLocalContext *lc); bool ff_vvc_palette_transpose_flag(VVCLocalContext *lc); bool ff_vvc_run_copy_flag(VVCLocalContext *lc, int prev_run_type, int prev_run_position, int cur_pos); bool ff_vvc_copy_above_palette_indices_flag(VVCLocalContext *lc); int ff_vvc_palette_idx_idc(VVCLocalContext *lc, int max_palette_index, bool adjust); -int ff_vvc_palette_escape_val(VVCLocalContext *lc); +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max); //inter int ff_vvc_general_merge_flag(VVCLocalContext *lc); diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c index 35c18e78f6..9f875d0a20 100644 --- a/libavcodec/vvc/ctu.c +++ b/libavcodec/vvc/ctu.c @@ -1857,16 +1857,16 @@ static int palette_predicted(VVCLocalContext *lc, const bool local_dual_tree, in } for (int i = 0; i < predictor_size && nb_predicted < max_entries; i++) { - const int run = ff_vvc_palette_predictor_run(lc); + const int run = ff_vvc_palette_predictor_run(lc, predictor_size - i); + if (run < 0) + return run; + if (run == 1) break; if (run > 1) i += run - 1; - if (i >= predictor_size) - return AVERROR_INVALIDDATA; - predictor_reused[i] = true; for (int c = start; c < end; c++) cu->plt[c].entries[nb_predicted] = lc->ep->pp[c].entries[i]; @@ -1885,12 +1885,17 @@ static int palette_signaled(VVCLocalContext *lc, const bool local_dual_tree, const VVCSPS *sps = lc->fc->ps.sps; CodingUnit *cu = lc->cu; const int nb_predicted = cu->plt[start].size; - const int nb_signaled = nb_predicted < max_entries ? ff_vvc_num_signalled_palette_entries(lc) : 0; - const int size = nb_predicted + nb_signaled; const bool dual_tree_luma = local_dual_tree && cu->tree_type == DUAL_TREE_LUMA; + int nb_signaled, size; - if (size > max_entries || nb_signaled < 0) - return AVERROR_INVALIDDATA; + if (nb_predicted < max_entries) { + nb_signaled = ff_vvc_num_signalled_palette_entries(lc, max_entries - nb_predicted); + if (nb_signaled < 0) + return nb_signaled; + } else + nb_signaled = 0; + + size = nb_predicted + nb_signaled; for (int c = start; c < end; c++) { Palette *plt = cu->plt + c; @@ -2052,10 +2057,11 @@ static int palette_subblock_data(VVCLocalContext *lc, if (!(xc & hs) && !(yc & vs)) { const int v = PALETTE_INDEX(xc, yc); if (v == esc) { - const int coeff = ff_vvc_palette_escape_val(lc); - if (coeff >= (1U << sps->bit_depth)) - return AVERROR_INVALIDDATA; - const int pixel = av_clip_intp2(RSHIFT(coeff * scale, 6), sps->bit_depth); + int pixel; + const int coeff = ff_vvc_palette_escape_val(lc, (1 << sps->bit_depth) - 1); + if (coeff < 0) + return coeff; + pixel = av_clip_intp2(RSHIFT(coeff * scale, 6), sps->bit_depth); PALETTE_SET_PIXEL(xc, yc, pixel); } else { PALETTE_SET_PIXEL(xc, yc, plt->entries[v]); -- 2.47.0 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Add max parameter to kth_order_egk_decode 2025-07-15 18:48 ` [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Add max parameter to kth_order_egk_decode Frank Plowman @ 2025-07-16 10:08 ` Frank Plowman 0 siblings, 0 replies; 3+ messages in thread From: Frank Plowman @ 2025-07-16 10:08 UTC (permalink / raw) To: ffmpeg-devel [-- Attachment #1.1.1.1: Type: text/plain, Size: 8002 bytes --] On 15/07/2025 03:48, Frank Plowman wrote: > Prior to this patch, kth_order_egk_decode could read arbitrarily > large values which then overflowed and caused various issues. > Patch fixes this by making kth_order_egk_decode falliable, > requiring the caller to specify an upper bound and returning an > error if the read value would exceed that bound. > > This patch resolves the same issue as > eb52251c0ab025b6b40b28994bc9dc616813b190, but I think this is the proper > fix as it also addresses issues with syntax elements besides > ff_vvc_num_signalled_palette_entries. > > Patch also includes a minor fix in hls_palette_coding, where the > error code returned by palette_subblock_data was previously unchecked. Spotted I forgot to remove the paragraph above from the commit message when splitting this aspect into a separate commit. I (or whoever else pushes) can remove this when commiting. > > Signed-off-by: Frank Plowman <post@frankplowman.com> > --- > Changes since v1: > * Split change to hls_palette_coding to its own commit. > * Return values from syntax functions in return val, rather than by > modifying pointer parameter. > --- > libavcodec/vvc/cabac.c | 19 ++++++++++++------- > libavcodec/vvc/cabac.h | 6 +++--- > libavcodec/vvc/ctu.c | 30 ++++++++++++++++++------------ > 3 files changed, 33 insertions(+), 22 deletions(-) > > diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c > index 6847ce59af..c2dbd46709 100644 > --- a/libavcodec/vvc/cabac.c > +++ b/libavcodec/vvc/cabac.c > @@ -929,7 +929,7 @@ static int truncated_binary_decode(VVCLocalContext *lc, const int c_max) > } > > // 9.3.3.5 k-th order Exp - Golomb binarization process > -static int kth_order_egk_decode(CABACContext *c, int k) > +static int kth_order_egk_decode(CABACContext *c, int k, const int max) > { > int bit = 1; > int value = 0; > @@ -937,6 +937,8 @@ static int kth_order_egk_decode(CABACContext *c, int k) > > while (bit) { > bit = get_cabac_bypass(c); > + if (max - value < (bit << k)) > + return AVERROR_INVALIDDATA; > value += bit << k++; > } > > @@ -946,6 +948,9 @@ static int kth_order_egk_decode(CABACContext *c, int k) > value += symbol; > } > > + if (value > max) > + return AVERROR_INVALIDDATA; > + > return value; > } > > @@ -1377,14 +1382,14 @@ int ff_vvc_intra_chroma_pred_mode(VVCLocalContext *lc) > return (get_cabac_bypass(&lc->ep->cc) << 1) | get_cabac_bypass(&lc->ep->cc); > } > > -int ff_vvc_palette_predictor_run(VVCLocalContext *lc) > +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max) > { > - return kth_order_egk_decode(&lc->ep->cc, 0); > + return kth_order_egk_decode(&lc->ep->cc, 0, max); > } > > -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc) > +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int max) > { > - return kth_order_egk_decode(&lc->ep->cc, 0); > + return kth_order_egk_decode(&lc->ep->cc, 0, max); > } > > int ff_vvc_new_palette_entries(VVCLocalContext *lc, const int bit_depth) > @@ -1424,9 +1429,9 @@ int ff_vvc_palette_idx_idc(VVCLocalContext *lc, const int max_palette_index, con > return truncated_binary_decode(lc, max_palette_index - adjust); > } > > -int ff_vvc_palette_escape_val(VVCLocalContext *lc) > +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max) > { > - return kth_order_egk_decode(&lc->ep->cc, 5); > + return kth_order_egk_decode(&lc->ep->cc, 5, max); > } > > int ff_vvc_general_merge_flag(VVCLocalContext *lc) > diff --git a/libavcodec/vvc/cabac.h b/libavcodec/vvc/cabac.h > index 972890317e..6a0e713d19 100644 > --- a/libavcodec/vvc/cabac.h > +++ b/libavcodec/vvc/cabac.h > @@ -81,15 +81,15 @@ int ff_vvc_intra_luma_mpm_remainder(VVCLocalContext *lc); > int ff_vvc_cclm_mode_flag(VVCLocalContext *lc); > int ff_vvc_cclm_mode_idx(VVCLocalContext *lc); > int ff_vvc_intra_chroma_pred_mode(VVCLocalContext *lc); > -int ff_vvc_palette_predictor_run(VVCLocalContext *lc); > -int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc); > +int ff_vvc_palette_predictor_run(VVCLocalContext *lc, const int max); > +int ff_vvc_num_signalled_palette_entries(VVCLocalContext *lc, const int max); > int ff_vvc_new_palette_entries(VVCLocalContext *lc, int bit_dpeth); > bool ff_vvc_palette_escape_val_present_flag(VVCLocalContext *lc); > bool ff_vvc_palette_transpose_flag(VVCLocalContext *lc); > bool ff_vvc_run_copy_flag(VVCLocalContext *lc, int prev_run_type, int prev_run_position, int cur_pos); > bool ff_vvc_copy_above_palette_indices_flag(VVCLocalContext *lc); > int ff_vvc_palette_idx_idc(VVCLocalContext *lc, int max_palette_index, bool adjust); > -int ff_vvc_palette_escape_val(VVCLocalContext *lc); > +int ff_vvc_palette_escape_val(VVCLocalContext *lc, const int max); > > //inter > int ff_vvc_general_merge_flag(VVCLocalContext *lc); > diff --git a/libavcodec/vvc/ctu.c b/libavcodec/vvc/ctu.c > index 35c18e78f6..9f875d0a20 100644 > --- a/libavcodec/vvc/ctu.c > +++ b/libavcodec/vvc/ctu.c > @@ -1857,16 +1857,16 @@ static int palette_predicted(VVCLocalContext *lc, const bool local_dual_tree, in > } > > for (int i = 0; i < predictor_size && nb_predicted < max_entries; i++) { > - const int run = ff_vvc_palette_predictor_run(lc); > + const int run = ff_vvc_palette_predictor_run(lc, predictor_size - i); > + if (run < 0) > + return run; > + > if (run == 1) > break; > > if (run > 1) > i += run - 1; > > - if (i >= predictor_size) > - return AVERROR_INVALIDDATA; > - > predictor_reused[i] = true; > for (int c = start; c < end; c++) > cu->plt[c].entries[nb_predicted] = lc->ep->pp[c].entries[i]; > @@ -1885,12 +1885,17 @@ static int palette_signaled(VVCLocalContext *lc, const bool local_dual_tree, > const VVCSPS *sps = lc->fc->ps.sps; > CodingUnit *cu = lc->cu; > const int nb_predicted = cu->plt[start].size; > - const int nb_signaled = nb_predicted < max_entries ? ff_vvc_num_signalled_palette_entries(lc) : 0; > - const int size = nb_predicted + nb_signaled; > const bool dual_tree_luma = local_dual_tree && cu->tree_type == DUAL_TREE_LUMA; > + int nb_signaled, size; > > - if (size > max_entries || nb_signaled < 0) > - return AVERROR_INVALIDDATA; > + if (nb_predicted < max_entries) { > + nb_signaled = ff_vvc_num_signalled_palette_entries(lc, max_entries - nb_predicted); > + if (nb_signaled < 0) > + return nb_signaled; > + } else > + nb_signaled = 0; > + > + size = nb_predicted + nb_signaled; > > for (int c = start; c < end; c++) { > Palette *plt = cu->plt + c; > @@ -2052,10 +2057,11 @@ static int palette_subblock_data(VVCLocalContext *lc, > if (!(xc & hs) && !(yc & vs)) { > const int v = PALETTE_INDEX(xc, yc); > if (v == esc) { > - const int coeff = ff_vvc_palette_escape_val(lc); > - if (coeff >= (1U << sps->bit_depth)) > - return AVERROR_INVALIDDATA; > - const int pixel = av_clip_intp2(RSHIFT(coeff * scale, 6), sps->bit_depth); > + int pixel; > + const int coeff = ff_vvc_palette_escape_val(lc, (1 << sps->bit_depth) - 1); > + if (coeff < 0) > + return coeff; > + pixel = av_clip_intp2(RSHIFT(coeff * scale, 6), sps->bit_depth); > PALETTE_SET_PIXEL(xc, yc, pixel); > } else { > PALETTE_SET_PIXEL(xc, yc, plt->entries[v]); [-- Attachment #1.1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 1091 bytes --] [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] [-- Attachment #2: Type: text/plain, Size: 251 bytes --] _______________________________________________ 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". ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-16 10:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-07-15 18:48 [FFmpeg-devel] [PATCH v2 1/2] lavc/vvc: Don't discard palette_subblock_data return code Frank Plowman 2025-07-15 18:48 ` [FFmpeg-devel] [PATCH v2 2/2] lavc/vvc: Add max parameter to kth_order_egk_decode Frank Plowman 2025-07-16 10:08 ` Frank Plowman
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git