[x264-devel] [PATCH] Fix index out-of-bound bugs

Vitaly Buka vitalybuka at gmail.com
Wed Apr 1 01:54:13 CEST 2020


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.11.0


More information about the x264-devel mailing list