[x264-devel] [PATCH] malloc result test

[Loïc Le Loarer]

Hi,

It seems that x264_malloc result is nearly never tested in allocation
functions, which could lead to segfaults in some situations.

Here a small patch which adds the correct tests for x264_frame_new
function with correct unallocation when something goes wrong. If this
patch is accepted, I'll go on to have this kind of test where necessary.

Best regards.

-- 
Loïc

"heaven is not a place, it's a feeling"
-------------- next part --------------
Index: common/frame.c
===================================================================
--- common/frame.c	(révision 527)
+++ common/frame.c	(copie de travail)
@@ -35,6 +35,10 @@
     int i_stride;
     int i_lines;
 
+    int b_failled_malloc = 0;
+
+    if ( NULL == frame ) return NULL;
+
     memset( frame, 0, sizeof(x264_frame_t) );
 
     /* allocate frame data (+64 for extra data for me) */
@@ -57,6 +61,7 @@
         frame->i_lines[i] = i_lines / i_divh;
         frame->buffer[i] = x264_malloc( frame->i_stride[i] *
                                         ( frame->i_lines[i] + 64 / i_divh ) );
+        b_failled_malloc |= ( NULL == frame->buffer[i] );
 
         frame->plane[i] = ((uint8_t*)frame->buffer[i]) +
                           frame->i_stride[i] * 32 / i_divh + 32 / i_divw;
@@ -71,6 +76,7 @@
     {
         frame->buffer[4+i] = x264_malloc( frame->i_stride[0] *
                                         ( frame->i_lines[0] + 64 ) );
+        b_failled_malloc |= ( NULL == frame->buffer[4+i] );
 
         frame->filtered[i+1] = ((uint8_t*)frame->buffer[4+i]) +
                                 frame->i_stride[0] * 32 + 32;
@@ -84,6 +90,7 @@
         {
             frame->buffer[7+i] = x264_malloc( frame->i_stride_lowres *
                                             ( frame->i_lines[0]/2 + 64 ) );
+            b_failled_malloc |= ( NULL == frame->buffer[7+i] );
             frame->lowres[i] = ((uint8_t*)frame->buffer[7+i]) +
                                 frame->i_stride_lowres * 32 + 32;
         }
@@ -92,6 +99,7 @@
     if( h->param.analyse.i_me_method == X264_ME_ESA )
     {
         frame->buffer[11] = x264_malloc( frame->i_stride[0] * (frame->i_lines[0] + 64) * sizeof(uint16_t) );
+        b_failled_malloc |= ( NULL == frame->buffer[11] );
         frame->integral = (uint16_t*)frame->buffer[11] + frame->i_stride[0] * 32 + 32;
     }
 
@@ -105,10 +113,15 @@
     frame->mb_type= x264_malloc( i_mb_count * sizeof( int8_t) );
     frame->mv[0]  = x264_malloc( 2*16 * i_mb_count * sizeof( int16_t ) );
     frame->ref[0] = x264_malloc( 4 * i_mb_count * sizeof( int8_t ) );
+    b_failled_malloc |= ( NULL == frame->mb_type )
+        || ( NULL == frame->mv[0] )
+        || ( NULL == frame->ref[0] );
     if( h->param.i_bframe )
     {
         frame->mv[1]  = x264_malloc( 2*16 * i_mb_count * sizeof( int16_t ) );
         frame->ref[1] = x264_malloc( 4 * i_mb_count * sizeof( int8_t ) );
+        b_failled_malloc |= ( NULL == frame->mv[1] )
+            || ( NULL == frame->ref[1] );
     }
     else
     {
@@ -118,20 +131,28 @@
 
     frame->i_row_bits = x264_malloc( i_lines/16 * sizeof( int ) );
     frame->i_row_qp   = x264_malloc( i_lines/16 * sizeof( int ) );
+    b_failled_malloc |= ( NULL == frame->i_row_bits )
+        || ( NULL == frame->i_row_qp );
     for( i = 0; i < h->param.i_bframe + 2; i++ )
         for( j = 0; j < h->param.i_bframe + 2; j++ )
+        {
             frame->i_row_satds[i][j] = x264_malloc( i_lines/16 * sizeof( int ) );
+            b_failled_malloc |= ( NULL == frame->i_row_satds[i][j] );
+        }
 
+    if (b_failled_malloc)
+    {
+        x264_frame_delete( frame );
+        return NULL;
+    }
     return frame;
 }
 
 void x264_frame_delete( x264_frame_t *frame )
 {
     int i, j;
-    for( i = 0; i < frame->i_plane; i++ )
+    for( i = 0; i < 12; i++ )
         x264_free( frame->buffer[i] );
-    for( i = 4; i < 12; i++ ) /* filtered planes */
-        x264_free( frame->buffer[i] );
     for( i = 0; i < X264_BFRAME_MAX+2; i++ )
         for( j = 0; j < X264_BFRAME_MAX+2; j++ )
             x264_free( frame->i_row_satds[i][j] );
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.videolan.org/pipermail/x264-devel/attachments/20060524/37ad64a7/attachment.pgp