[x264-devel] [PATCH] Fix index out-of-bound bugs
Vitaly Buka
vitalybuka at google.com
Wed Apr 1 02:12:53 CEST 2020
From: Vitaly Buka <vitalybuka at gmail.com>
1. i_mb_partition does not match loop at encoder/encoder.c:4025
It looks like it should have the same size as i_mb_count[3].
2. lowres_mvs[1][p1-b-1] is fix of "undefined behaviour"
In some cases index is negative. There is no memory when it happens.
However, according to the C standard even calculation of such pointer
is undefined behaviour. https://port70.net/~nsz/c/c11/n1570.html#6.5.6p8
Both were detected with clang -fsanitize=bounds and the fix will help
to use this code in projects where -fsanitize=bounds is used.
---
common/common.h | 2 +-
encoder/slicetype.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/common/common.h b/common/common.h
index a167df63..06f9144c 100644
--- a/common/common.h
+++ b/common/common.h
@@ -697,7 +697,7 @@ struct x264_t
double f_frame_duration[3];
/* */
int64_t i_mb_count[3][19];
- int64_t i_mb_partition[2][17];
+ int64_t i_mb_partition[3][17];
int64_t i_mb_count_8x8dct[2];
int64_t i_mb_count_ref[2][2][X264_REF_MAX*2];
int64_t i_mb_cbp[6];
diff --git a/encoder/slicetype.c b/encoder/slicetype.c
index a520f586..487727ee 100644
--- a/encoder/slicetype.c
+++ b/encoder/slicetype.c
@@ -519,8 +519,6 @@ static void slicetype_mb_cost( x264_t *h, x264_mb_analysis_t *a,
const int i_stride = fenc->i_stride_lowres;
const int i_pel_offset = 8 * (i_mb_x + i_mb_y * i_stride);
const int i_bipred_weight = h->param.analyse.b_weighted_bipred ? 64 - (dist_scale_factor>>2) : 32;
- int16_t (*fenc_mvs[2])[2] = { &fenc->lowres_mvs[0][b-p0-1][i_mb_xy], &fenc->lowres_mvs[1][p1-b-1][i_mb_xy] };
- int (*fenc_costs[2]) = { &fenc->lowres_mv_costs[0][b-p0-1][i_mb_xy], &fenc->lowres_mv_costs[1][p1-b-1][i_mb_xy] };
int b_frame_score_mb = (i_mb_x > 0 && i_mb_x < h->mb.i_mb_width - 1 &&
i_mb_y > 0 && i_mb_y < h->mb.i_mb_height - 1) ||
h->mb.i_mb_width <= 2 || h->mb.i_mb_height <= 2;
@@ -539,6 +537,9 @@ static void slicetype_mb_cost( x264_t *h, x264_mb_analysis_t *a,
if( p0 == p1 )
goto lowres_intra_mb;
+ int16_t (*fenc_mvs[2])[2] = { &fenc->lowres_mvs[0][b-p0-1][i_mb_xy], p1 == b ? NULL : &fenc->lowres_mvs[1][p1-b-1][i_mb_xy] };
+ int (*fenc_costs[2]) = { &fenc->lowres_mv_costs[0][b-p0-1][i_mb_xy], p1 == b ? NULL : &fenc->lowres_mv_costs[1][p1-b-1][i_mb_xy] };
+
int mv_range = 2 * h->param.analyse.i_mv_range;
// no need for h->mb.mv_min[]
h->mb.mv_min_spel[0] = X264_MAX( 4*(-8*h->mb.i_mb_x - 12), -mv_range );
@@ -1045,7 +1046,7 @@ static void macroblock_tree_propagate( x264_t *h, x264_frame_t **frames, float a
uint16_t *ref_costs[2] = {frames[p0]->i_propagate_cost,frames[p1]->i_propagate_cost};
int dist_scale_factor = ( ((b-p0) << 8) + ((p1-p0) >> 1) ) / (p1-p0);
int i_bipred_weight = h->param.analyse.b_weighted_bipred ? 64 - (dist_scale_factor>>2) : 32;
- int16_t (*mvs[2])[2] = { frames[b]->lowres_mvs[0][b-p0-1], frames[b]->lowres_mvs[1][p1-b-1] };
+ int16_t (*mvs[2])[2] = { frames[b]->lowres_mvs[0][b-p0-1], b == p1 ? NULL : frames[b]->lowres_mvs[1][p1-b-1] };
int bipred_weights[2] = {i_bipred_weight, 64 - i_bipred_weight};
int16_t *buf = h->scratch_buffer;
uint16_t *propagate_cost = frames[b]->i_propagate_cost;
--
2.26.0.rc2.310.g2932bb562d-goog
More information about the x264-devel
mailing list