[x264-devel] [Git][videolan/x264][master] 3 commits: Fix integer overflow in cabac psy-trellis

Anton Mitrofanov (@BugMaster) gitlab at videolan.org
Wed Sep 29 21:28:42 UTC 2021



Anton Mitrofanov pushed to branch master at VideoLAN / x264


Commits:
3a08e36e by Anton Mitrofanov at 2021-09-29T21:25:26+00:00
Fix integer overflow in cabac psy-trellis

- - - - -
78520340 by Anton Mitrofanov at 2021-09-29T21:25:26+00:00
Fix integer overflow in cavlc trellis

- - - - -
5bb1f1b1 by Anton Mitrofanov at 2021-09-29T21:25:26+00:00
Fix implicit integer sign change and truncation

- - - - -


9 changed files:

- common/bitstream.c
- common/cabac.c
- common/mc.h
- common/mvpred.c
- common/rectangle.h
- common/vlc.c
- common/x86/util.h
- encoder/rdo.c
- output/matroska_ebml.c


Changes:

=====================================
common/bitstream.c
=====================================
@@ -92,10 +92,10 @@ void x264_nal_encode( x264_t *h, uint8_t *dst, x264_nal_t *nal )
     {
         /* Size doesn't include the size of the header we're writing now. */
         int chunk_size = size - 4;
-        orig_dst[0] = chunk_size >> 24;
-        orig_dst[1] = chunk_size >> 16;
-        orig_dst[2] = chunk_size >> 8;
-        orig_dst[3] = chunk_size >> 0;
+        orig_dst[0] = (uint8_t)(chunk_size >> 24);
+        orig_dst[1] = (uint8_t)(chunk_size >> 16);
+        orig_dst[2] = (uint8_t)(chunk_size >> 8);
+        orig_dst[3] = (uint8_t)(chunk_size >> 0);
     }
 
     nal->i_payload = size;


=====================================
common/cabac.c
=====================================
@@ -89,10 +89,10 @@ static inline void cabac_putbyte( x264_cabac_t *cb )
             cb->p[-1] += carry;
             while( bytes_outstanding > 0 )
             {
-                *(cb->p++) = carry-1;
+                *(cb->p++) = (uint8_t)(carry-1);
                 bytes_outstanding--;
             }
-            *(cb->p++) = out;
+            *(cb->p++) = (uint8_t)out;
             cb->i_bytes_outstanding = 0;
         }
     }


=====================================
common/mc.h
=====================================
@@ -61,8 +61,8 @@ static void mbtree_propagate_list_##cpu( x264_t *h, uint16_t *ref_costs, int16_t
             if( !(lowres_costs[i] & (1 << (list+LOWRES_COST_SHIFT))) )\
                 continue;\
 \
-            unsigned mbx = current[0];\
-            unsigned mby = current[1];\
+            unsigned mbx = (unsigned)current[0];\
+            unsigned mby = (unsigned)current[1];\
             unsigned idx0 = mbx + mby * stride;\
             unsigned idx2 = idx0 + stride;\
 \


=====================================
common/mvpred.c
=====================================
@@ -171,8 +171,8 @@ void x264_mb_predict_mv_pskip( x264_t *h, int16_t mv[2] )
     int16_t *mv_b  = h->mb.cache.mv[0][X264_SCAN8_0 - 8];
 
     if( i_refa == -2 || i_refb == -2 ||
-        !( i_refa | M32( mv_a ) ) ||
-        !( i_refb | M32( mv_b ) ) )
+        !( (uint32_t)i_refa | M32( mv_a ) ) ||
+        !( (uint32_t)i_refb | M32( mv_b ) ) )
     {
         M32( mv ) = 0;
     }
@@ -304,7 +304,7 @@ static ALWAYS_INLINE int mb_predict_mv_direct16x16_spatial( x264_t *h, int b_int
             mv_c   = h->mb.cache.mv[i_list][X264_SCAN8_0 - 8 - 1];
         }
 
-        int i_ref = X264_MIN3( (unsigned)i_refa, (unsigned)i_refb, (unsigned)i_refc );
+        int i_ref = (int)X264_MIN3( (unsigned)i_refa, (unsigned)i_refb, (unsigned)i_refc );
         if( i_ref < 0 )
         {
             i_ref = -1;


=====================================
common/rectangle.h
=====================================
@@ -28,8 +28,8 @@
 static ALWAYS_INLINE void x264_macroblock_cache_rect( void *dst, int w, int h, int s, uint32_t v )
 {
     uint8_t *d = dst;
-    uint16_t v2 = s == 2 ? v : v * 0x101;
-    uint32_t v4 = s == 4 ? v : s == 2 ? v * 0x10001 : v * 0x1010101;
+    uint16_t v2 = s >= 2 ? v : v * 0x101;
+    uint32_t v4 = s >= 4 ? v : s >= 2 ? v * 0x10001 : v * 0x1010101;
     uint64_t v8 = v4 + ((uint64_t)v4 << 32);
     s *= 8;
 
@@ -142,13 +142,13 @@ static ALWAYS_INLINE void x264_macroblock_cache_mvd( x264_t *h, int x, int y, in
     else
         x264_macroblock_cache_rect( mvd_cache, width*2, height, 2, mvd );
 }
-static ALWAYS_INLINE void x264_macroblock_cache_ref( x264_t *h, int x, int y, int width, int height, int i_list, uint8_t ref )
+static ALWAYS_INLINE void x264_macroblock_cache_ref( x264_t *h, int x, int y, int width, int height, int i_list, int8_t ref )
 {
     void *ref_cache = &h->mb.cache.ref[i_list][X264_SCAN8_0+x+8*y];
     if( x264_nonconstant_p( width ) || x264_nonconstant_p( height ) )
-        x264_cache_ref_func_table[width + (height<<1)-3]( ref_cache, ref );
+        x264_cache_ref_func_table[width + (height<<1)-3]( ref_cache, (uint8_t)ref );
     else
-        x264_macroblock_cache_rect( ref_cache, width, height, 1, ref );
+        x264_macroblock_cache_rect( ref_cache, width, height, 1, (uint8_t)ref );
 }
 static ALWAYS_INLINE void x264_macroblock_cache_skip( x264_t *h, int x, int y, int width, int height, int b_skip )
 {


=====================================
common/vlc.c
=====================================
@@ -37,7 +37,7 @@ void x264_cavlc_init( x264_t *h )
         {
             int mask = level >> 15;
             int abs_level = (level^mask)-mask;
-            int i_level_code = abs_level*2-mask-2;
+            int i_level_code = abs_level ? abs_level*2-mask-2 : 0;
             int i_next = i_suffix;
             vlc_large_t *vlc = &x264_level_token[i_suffix][level+LEVEL_TABLE_SIZE/2];
 


=====================================
common/x86/util.h
=====================================
@@ -121,7 +121,7 @@ static ALWAYS_INLINE uint16_t x264_cabac_mvd_sum_mmx2(uint8_t *mvdleft, uint8_t
          "m"(pb_2),"m"(pb_32),"m"(pb_33)
         :"mm0", "mm1", "mm2"
     );
-    return amvd;
+    return (uint16_t)amvd;
 }
 
 #define x264_predictor_clip x264_predictor_clip_mmx2


=====================================
encoder/rdo.c
=====================================
@@ -471,7 +471,7 @@ int trellis_dc_shortcut( int sign_coef, int quant_coef, int unquant_mf, int coef
 
         /* Optimize rounding for DC coefficients in DC-only luma 4x4/8x8 blocks. */
         int d = sign_coef - ((SIGN(unquant_abs_level, sign_coef) + 8)&~15);
-        uint64_t score = (uint64_t)d*d * coef_weight;
+        uint64_t score = (int64_t)d*d * coef_weight;
 
         /* code the proposed level, and count how much entropy it would take */
         if( abs_level )
@@ -734,11 +734,11 @@ int quant_trellis_cabac( x264_t *h, dctcoef *dct,
     trellis_level_t level_tree[64*8*2];
     int levels_used = 1;
     /* init trellis */
-    trellis_node_t nodes[2][8];
+    trellis_node_t nodes[2][8] = {0};
     trellis_node_t *nodes_cur = nodes[0];
     trellis_node_t *nodes_prev = nodes[1];
     trellis_node_t *bnode;
-    for( int j = 1; j < 4; j++ )
+    for( int j = 1; j < 8; j++ )
         nodes_cur[j].score = TRELLIS_SCORE_MAX;
     nodes_cur[0].score = TRELLIS_SCORE_BIAS;
     nodes_cur[0].level_idx = 0;
@@ -825,17 +825,18 @@ int quant_trellis_cabac( x264_t *h, dctcoef *dct,
                 int predicted_coef = orig_coef - sign_coef;\
                 int psy_value = abs(unquant_abs_level + SIGN(predicted_coef, sign_coef));\
                 int psy_weight = coef_weight1[zigzag[i]] * h->mb.i_psy_trellis;\
-                ssd1[k] = (uint64_t)d*d * coef_weight2[zigzag[i]] - psy_weight * psy_value;\
+                int64_t tmp = (int64_t)d*d * coef_weight2[zigzag[i]] - (int64_t)psy_weight * psy_value;\
+                ssd1[k] = (uint64_t)tmp;\
             }\
             else\
             /* FIXME: for i16x16 dc is this weight optimal? */\
-                ssd1[k] = (uint64_t)d*d * (dc?256:coef_weight2[zigzag[i]]);\
+                ssd1[k] = (int64_t)d*d * (dc?256:coef_weight2[zigzag[i]]);\
             ssd0[k] = ssd1[k];\
             if( !i && !dc && !ctx_hi )\
             {\
                 /* Optimize rounding for DC coefficients in DC-only luma 4x4/8x8 blocks. */\
                 d = sign_coef - ((SIGN(unquant_abs_level, sign_coef) + 8)&~15);\
-                ssd0[k] = (uint64_t)d*d * coef_weight2[zigzag[i]];\
+                ssd0[k] = (int64_t)d*d * coef_weight2[zigzag[i]];\
             }\
         }\
 \
@@ -926,7 +927,7 @@ int quant_trellis_cavlc( x264_t *h, dctcoef *dct,
     ALIGNED_ARRAY_16( dctcoef, coefs,[16] );
     const uint32_t *coef_weight1 = b_8x8 ? x264_dct8_weight_tab : x264_dct4_weight_tab;
     const uint32_t *coef_weight2 = b_8x8 ? x264_dct8_weight2_tab : x264_dct4_weight2_tab;
-    int delta_distortion[16];
+    int64_t delta_distortion[16];
     int64_t score = 1ULL<<62;
     int i, j;
     const int f = 1<<15;
@@ -953,7 +954,7 @@ int quant_trellis_cavlc( x264_t *h, dctcoef *dct,
 
     /* Find last non-zero coefficient. */
     for( i = end; i >= start; i -= step )
-        if( (unsigned)(dct[zigzag[i]] * (dc?quant_mf[0]>>1:quant_mf[zigzag[i]]) + f-1) >= 2*f )
+        if( abs(dct[zigzag[i]]) * (dc?quant_mf[0]>>1:quant_mf[zigzag[i]]) >= f )
             break;
 
     if( i < start )
@@ -986,7 +987,7 @@ int quant_trellis_cavlc( x264_t *h, dctcoef *dct,
             int unquant0 = (((dc?unquant_mf[0]<<1:unquant_mf[zigzag[j]]) * (nearest_quant-1) + 128) >> 8);
             int d1 = abs_coef - unquant1;
             int d0 = abs_coef - unquant0;
-            delta_distortion[i] = (d0*d0 - d1*d1) * (dc?256:coef_weight2[zigzag[j]]);
+            delta_distortion[i] = (int64_t)(d0*d0 - d1*d1) * (dc?256:coef_weight2[zigzag[j]]);
 
             /* Psy trellis: bias in favor of higher AC coefficients in the reconstructed frame. */
             if( h->mb.i_psy_trellis && j && !dc && !b_chroma )
@@ -1024,7 +1025,7 @@ int quant_trellis_cavlc( x264_t *h, dctcoef *dct,
     while( 1 )
     {
         int64_t iter_score = score;
-        int iter_distortion_delta = 0;
+        int64_t iter_distortion_delta = 0;
         int iter_coef = -1;
         int iter_mask = coef_mask;
         int iter_round = round_mask;
@@ -1039,7 +1040,7 @@ int quant_trellis_cavlc( x264_t *h, dctcoef *dct,
             int old_coef = coefs[i];
             int new_coef = quant_coefs[round_change][i];
             int cur_mask = (coef_mask&~(1 << i))|(!!new_coef << i);
-            int cur_distortion_delta = delta_distortion[i] * (round_change ? -1 : 1);
+            int64_t cur_distortion_delta = delta_distortion[i] * (round_change ? -1 : 1);
             int64_t cur_score = cur_distortion_delta;
             coefs[i] = new_coef;
 


=====================================
output/matroska_ebml.c
=====================================
@@ -60,7 +60,7 @@ struct mk_writer
     int64_t cluster_tc_scaled;
     int64_t frame_tc, max_frame_tc;
 
-    char wrote_header, in_frame, keyframe, skippable;
+    int8_t wrote_header, in_frame, keyframe, skippable;
 };
 
 static mk_context *mk_create_context( mk_writer *w, mk_context *parent, unsigned id )
@@ -111,7 +111,7 @@ static int mk_append_context_data( mk_context *c, const void *data, unsigned siz
         c->d_max = dn;
     }
 
-    memcpy( (char*)c->data + c->d_cur, data, size );
+    memcpy( (uint8_t*)c->data + c->d_cur, data, size );
 
     c->d_cur = ns;
 
@@ -120,7 +120,7 @@ static int mk_append_context_data( mk_context *c, const void *data, unsigned siz
 
 static int mk_write_id( mk_context *c, unsigned id )
 {
-    unsigned char c_id[4] = { id >> 24, id >> 16, id >> 8, id };
+    uint8_t c_id[4] = { id >> 24, id >> 16, id >> 8, id };
 
     if( c_id[0] )
         return mk_append_context_data( c, c_id, 4 );
@@ -133,7 +133,7 @@ static int mk_write_id( mk_context *c, unsigned id )
 
 static int mk_write_size( mk_context *c, unsigned size )
 {
-    unsigned char c_size[5] = { 0x08, size >> 24, size >> 16, size >> 8, size };
+    uint8_t c_size[5] = { 0x08, size >> 24, size >> 16, size >> 8, size };
 
     if( size < 0x7f )
     {
@@ -160,7 +160,7 @@ static int mk_write_size( mk_context *c, unsigned size )
 
 static int mk_flush_context_id( mk_context *c )
 {
-    unsigned char ff = 0xff;
+    uint8_t ff = 0xff;
 
     if( !c->id )
         return 0;
@@ -249,9 +249,9 @@ static int mk_write_bin( mk_context *c, unsigned id, const void *data, unsigned
     return 0;
 }
 
-static int mk_write_uint( mk_context *c, unsigned id, int64_t ui )
+static int mk_write_uint( mk_context *c, unsigned id, uint64_t ui )
 {
-    unsigned char c_ui[8] = { ui >> 56, ui >> 48, ui >> 40, ui >> 32, ui >> 24, ui >> 16, ui >> 8, ui };
+    uint8_t c_ui[8] = { ui >> 56, ui >> 48, ui >> 40, ui >> 32, ui >> 24, ui >> 16, ui >> 8, ui };
     unsigned i = 0;
 
     CHECK( mk_write_id( c, id ) );
@@ -267,9 +267,9 @@ static int mk_write_float_raw( mk_context *c, float f )
     union
     {
         float f;
-        unsigned u;
+        uint32_t u;
     } u;
-    unsigned char c_f[4];
+    uint8_t c_f[4];
 
     u.f = f;
     c_f[0] = u.u >> 24;
@@ -408,7 +408,7 @@ static int mk_flush_frame( mk_writer *w )
 {
     int64_t delta;
     unsigned fsize;
-    unsigned char c_delta_flags[3];
+    uint8_t c_delta_flags[3];
 
     if( !w->in_frame )
         return 0;
@@ -435,8 +435,8 @@ static int mk_flush_frame( mk_writer *w )
     CHECK( mk_write_size( w->cluster, fsize + 4 ) ); // Size
     CHECK( mk_write_size( w->cluster, 1 ) ); // TrackNumber
 
-    c_delta_flags[0] = delta >> 8;
-    c_delta_flags[1] = delta;
+    c_delta_flags[0] = (uint8_t)(delta >> 8);
+    c_delta_flags[1] = (uint8_t)delta;
     c_delta_flags[2] = (w->keyframe << 7) | w->skippable;
     CHECK( mk_append_context_data( w->cluster, c_delta_flags, 3 ) ); // Timecode, Flags
     if( w->frame )



View it on GitLab: https://code.videolan.org/videolan/x264/-/compare/6c142bba65bd864bab4c8d8593c510ce29d6d6d9...5bb1f1b1bad1ee6be2437de9ae3ed4b87095f6f4

-- 
View it on GitLab: https://code.videolan.org/videolan/x264/-/compare/6c142bba65bd864bab4c8d8593c510ce29d6d6d9...5bb1f1b1bad1ee6be2437de9ae3ed4b87095f6f4
You're receiving this email because of your account on code.videolan.org.




More information about the x264-devel mailing list