The attached patch addresses minor buffer-overflow issues and potential
buffer-overflow issues in the memory-logging code.
1) The LZ-compress function have been modified to accept an output-buffer size.
The header file and the memory-logging function that invokes the compress
function have been modified accordingly.
2) A number of buffer overflow issues have been fixed in the memlog_write
function in tboot/common/printk.c. The issues are mentioned below. In addition,
comments have been added and the code modified to improve readability:
a) At the start of the function, g_log->zip_count may be ZIP_COUNT_MAX. Line
105 reads (g_log->zip_pos[g_log->zip_count]) without checking g_log->zip_count.
Line 106 reads (g_log->zip_pos[g_log->zip_count]) before checking
g_log->zip_count.
b) Line 109 increments g_log->zip_count. After this point, g_log->zip_count may
be ZIP_COUNT_MAX. Lines 111, 112, 113, and 115 read and write to
(g_log->zip_pos[g_log->zip_count]) without checking g_log->zip_count.
c) Line 99 compares (count) against (g_log->max_size). Line 103 compares
(g_log->curr_pos + count) to (g_log->max_size). Line 105 compares
(g_log->zip_pos[g_log->zip_count] + zip_size + count) to (g_log->max_size).
However, line 131 can potentially write a single NULL-terminator past the end
of the space that the previously mentioned lines check for (checking for
"count" but potentially writing "count+1").
Signed-off-by: Safayet Ahmed <safayet.ah...@ge.com<mailto:safayet.ah...@ge.com>>
thanks,
Safayet
diff -r 4b75947a9646 tboot/common/lz.c
--- a/tboot/common/lz.c Fri Jun 17 17:52:42 2016 -0700
+++ b/tboot/common/lz.c Tue Jul 05 10:00:37 2016 -0400
@@ -113,7 +113,7 @@
* _LZ_WriteVarSize() - Write unsigned integer with variable number of
* bytes depending on value.
*************************************************************************/
-
+/*will write at most 5 bytes to buf*/
static int _LZ_WriteVarSize( unsigned int x, char * buf )
{
unsigned int y;
@@ -184,10 +184,12 @@
* out - Output (compressed) buffer. This buffer must be 0.4% larger
* than the input buffer, plus one byte.
* insize - Number of input bytes.
-* The function returns the size of the compressed data.
+* outsize - Nuber of bytes that can be stored in the output buffer.
+* The function returns the size of the compressed data or (-1) if there
+* is insufficient space in the output buffer.
*************************************************************************/
-int LZ_Compress( char *in, char *out, unsigned int insize )
+int LZ_Compress( char *in, char *out, unsigned int insize, unsigned int outsize)
{
char marker, symbol;
unsigned int inpos, outpos, bytesleft, i;
@@ -202,6 +204,11 @@
return 0;
}
+ if( outsize < 1 )
+ {
+ return -1;
+ }
+
/* Create histogram */
for( i = 0; i < 256; ++ i )
{
@@ -271,6 +278,9 @@
((bestlength == 6) && (bestoffset <= 0x001fffff)) ||
((bestlength == 7) && (bestoffset <= 0x0fffffff)) )
{
+ if( (outpos + 1 + 5 + 5) > outsize )
+ return -1;
+
out[ outpos ++ ] = (char) marker;
outpos += _LZ_WriteVarSize( bestlength, &out[ outpos ] );
outpos += _LZ_WriteVarSize( bestoffset, &out[ outpos ] );
@@ -279,6 +289,9 @@
}
else
{
+ if( (outpos + 2) > outsize )
+ return -1;
+
/* Output single byte (or two bytes if marker byte) */
symbol = in[ inpos ++ ];
out[ outpos ++ ] = symbol;
@@ -292,6 +305,9 @@
while( bytesleft > 3 );
/* Dump remaining bytes, if any */
+ if( (outpos + bytesleft*2) > outsize )
+ return -1;
+
while( inpos < insize )
{
if( in[ inpos ] == marker )
diff -r 4b75947a9646 tboot/common/printk.c
--- a/tboot/common/printk.c Fri Jun 17 17:52:42 2016 -0700
+++ b/tboot/common/printk.c Tue Jul 05 10:00:37 2016 -0400
@@ -91,32 +91,69 @@
/* allocate a 32K temp buffer for compressed log */
static char buf[32*1024];
char *out=buf;
- uint32_t zip_size;
+ int zip_size;
+ uint32_t zip_pos;
+ bool log_reset_flag;
if ( g_log == NULL || count > g_log->max_size )
return;
- /* do a compression of the log if curr_pos + count > max_size */
- if (g_log->curr_pos + count > g_log->max_size) {
- zip_size = LZ_Compress(&g_log->buf[g_log->zip_pos[g_log->zip_count]], out, g_log->curr_pos - g_log->zip_pos[g_log->zip_count]);
- if ((g_log->zip_pos[g_log->zip_count] + zip_size + count < g_log->max_size) && (g_log->zip_count < ZIP_COUNT_MAX)) {
- memcpy(&g_log->buf[g_log->zip_pos[g_log->zip_count]], out, zip_size);
- g_log->zip_size[g_log->zip_count] = zip_size;
- g_log->zip_count++;
- g_log->zip_pos[g_log->zip_count] = g_log->zip_pos[g_log->zip_count - 1] + zip_size;
- g_log->curr_pos = g_log->zip_pos[g_log->zip_count];
- if (g_log->buf[g_log->zip_pos[g_log->zip_count] - 1] !='\0')
- g_log->buf[g_log->zip_pos[g_log->zip_count]] = '\0';
- else {
- g_log->zip_pos[g_log->zip_count]--;
- g_log->curr_pos--;
+ /* Check if there is space for the new string and a null terminator */
+ if (g_log->curr_pos + count + 1> g_log->max_size) {
+
+ /* If there are space issues, only then log will be reset */
+ log_reset_flag = false;
+
+ /* Check if there is space to add another compressed chunk */
+ if(g_log->zip_count >= ZIP_COUNT_MAX)
+ log_reset_flag = true;
+ else{
+ /* Get the start position of the new compressed chunk */
+ zip_pos = g_log->zip_pos[g_log->zip_count];
+
+ /* Compress the last part of the log buffer that is not compressed,
+ and put the compressed output in out (buf) */
+ zip_size = LZ_Compress(&g_log->buf[zip_pos], out, (g_log->curr_pos - zip_pos), sizeof(buf) );
+
+ /* Check if buf was large enough for LZ_compress to succeed */
+ if( zip_size < 0 )
+ log_reset_flag = true;
+ else{
+ /* Check if there is space to add the compressed string, the
+ new string and a null terminator to the log */
+ if( (zip_pos + zip_size + count + 1) > g_log->max_size )
+ log_reset_flag = true;
+ else{
+ /* Add the new compressed chunk to the log buffer,
+ over-writing the last part of the log that was just
+ compressed */
+ memcpy(&g_log->buf[zip_pos], out, zip_size);
+ g_log->zip_size[g_log->zip_count] = zip_size;
+ g_log->zip_count++;
+ g_log->curr_pos = zip_pos + zip_size;
+
+ /* If there was no NULL terminator at the end of the
+ compressed chunk, add one. In either case,
+ g_log->curr_pos should point to it */
+ if (g_log->buf[g_log->curr_pos - 1] !='\0')
+ g_log->buf[g_log->curr_pos] ='\0';
+ else
+ g_log->curr_pos--;
+
+ /* Only if there is space to add another compressed chunk,
+ prepare its start position. */
+ if( g_log->zip_count < ZIP_COUNT_MAX )
+ g_log->zip_pos[g_log->zip_count] = g_log->curr_pos;
+ }
}
}
- else {
- g_log->curr_pos = 0;
- for ( uint8_t i = 0; i < ZIP_COUNT_MAX; i++ ) g_log->zip_pos[i] = 0;
- for ( uint8_t i = 0; i < ZIP_COUNT_MAX; i++ ) g_log->zip_size[i] = 0;
- g_log->zip_count = 0;
+
+ /* There was some space-shortage problem. Reset the log. */
+ if ( log_reset_flag ){
+ g_log->curr_pos = 0;
+ for( uint8_t i = 0; i < ZIP_COUNT_MAX; i++ ) g_log->zip_pos[i] = 0;
+ for( uint8_t i = 0; i < ZIP_COUNT_MAX; i++ ) g_log->zip_size[i] = 0;
+ g_log->zip_count = 0;
}
}
diff -r 4b75947a9646 tboot/include/lz.h
--- a/tboot/include/lz.h Fri Jun 17 17:52:42 2016 -0700
+++ b/tboot/include/lz.h Tue Jul 05 10:00:37 2016 -0400
@@ -41,7 +41,7 @@
* Function prototypes
*************************************************************************/
-int LZ_Compress( char *in, char *out, unsigned int insize );
+int LZ_Compress( char *in, char *out, unsigned int insize, unsigned int outsize);
void LZ_Uncompress( char *in, char *out, unsigned int insize );
------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
tboot-devel mailing list
tboot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tboot-devel
