PRD: buffer overrun issue in PRD capture data

Change-Id: I720c84c77cef6029501596b5dd4ba7dc7c2f8fbb
CQ: SW453391
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/70563
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Reviewed-by: Caleb N. Palmer <cnpalmer@us.ibm.com>
Reviewed-by: Brian J. Stegmiller <bjs@us.ibm.com>
Reviewed-by: Zane C. Shelley <zshelle@us.ibm.com>
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/70704
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>

4 files changed, 145 insertions, 115 deletions

diff --git a/src/usr/diag/prdf/common/framework/register/iipCaptureData.h b/src/usr/diag/prdf/common/framework/register/iipCaptureData.h
index f1a9d4d35..e65e94d3f 100755
--- a/src/usr/diag/prdf/common/framework/register/iipCaptureData.h
+++ b/src/usr/diag/prdf/common/framework/register/iipCaptureData.h
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2012,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2012,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -247,19 +247,18 @@ public:
 void Drop(RegType type);  //@jl04a
 // end @jl04a
 
-  /**
-   Copy caputre data to buffer
-   <ul>
-   <br><b>Parameter:  ptr to buffer to place capture data
-   <br><b>Parameter:  maxsize of buffer area
-   <br><b>Returns:   Returns the number of bytes copied
-   <br><b>Requirements: None
-   <br><b>Promises:  bytes copied <= bufferSize
-   <br><b>Notes:     Caputure data is placed in the buffer in the order it exists
-                     in the vector until done or buffer is full
-   <ul><br>
-   */
-  unsigned int Copy(uint8_t * buffer, unsigned int bufferSize) const;
+    /**
+     * @brief  Copies the capture data to a buffer.
+     *
+     * The capture data is copied to the buffer in the order it exists in the
+     * vector until all entries have been added or until the buffer is full.
+     *
+     * @param  i_buffer     Pointer to buffer.
+     * @param  i_bufferSize Maximum size of the buffer.
+     * @return The actual size of the data buffer. The value will always be less
+     *         than or equal to the maximum buffer size.
+     */
+    uint32_t Copy( uint8_t * i_buffer, uint32_t i_bufferSize ) const;
 
   // dg08a -->
   /**
diff --git a/src/usr/diag/prdf/common/framework/register/prdfCaptureData.C b/src/usr/diag/prdf/common/framework/register/prdfCaptureData.C
index 555160497..39113507b 100755
--- a/src/usr/diag/prdf/common/framework/register/prdfCaptureData.C
+++ b/src/usr/diag/prdf/common/framework/register/prdfCaptureData.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2012,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2012,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -155,104 +155,128 @@ void CaptureData::Drop(RegType i_type)
 
 //------------------------------------------------------------------------------
 
+template <class T>
+void __bufferAdd( uint8_t* & i_idx, T i_val )
+{
+    memcpy( i_idx, &i_val, sizeof(i_val) );
+    i_idx += sizeof(i_val);
+}
+
+bool __bufferFull( uint8_t * i_buf, size_t i_bufSize,
+                   uint8_t * i_idx, size_t i_idxSize )
+{
+    if ( (i_buf + i_bufSize) < (i_idx + i_idxSize) )
+    {
+        PRDF_ERR( "[CaptureData::Copy] Buffer is full. Some data may have "
+                  "been lost" );
+        return true;
+    }
+
+    return false;
+}
+
 /* CaptureData Format:
  *        capture data -> ( <chip header> <registers> )*
  *        chip header -> ( <chip id:32> <# registers:32> )
  *        registers -> ( <reg id:16> <reg byte len:16> <bytes>+ )
  */
-unsigned int CaptureData::Copy(uint8_t *i_buffer, unsigned int i_bufferSize) const
+uint32_t CaptureData::Copy( uint8_t * i_buffer, uint32_t i_bufferSize ) const
 {
-    TargetHandle_t  l_pcurrentChipHandle =NULL ;
-    uint8_t * l_entryCountPos = NULL;
-    uint32_t l_regEntries = 0;
+    TargetHandle_t curTrgt = nullptr;
 
-    uint32_t l_bytesWritten = 0;
-    for (ConstDataIterator i = data.begin(); i != data.end(); i++)
-    {
-        // Check for new chip.
-        if (i->chipHandle != l_pcurrentChipHandle)
-        {   // Update previous header, write new header.
+    uint32_t * regCntPtr = nullptr;
 
-            if (NULL != l_entryCountPos) // Update previous entry count.
-            {
-                l_regEntries = htonl(l_regEntries);
-                memcpy(l_entryCountPos, &l_regEntries, sizeof(l_regEntries));
-                l_regEntries = 0;
-            }
-
-            // Update chip Handles....
-            TargetHandle_t l_ptempHandle = l_pcurrentChipHandle = i->chipHandle;
-            HUID l_chipHuid =PlatServices::getHuid(l_ptempHandle);
-            const size_t l_huidSize = sizeof(l_chipHuid);
-            l_chipHuid = htonl(l_chipHuid);
+    uint8_t * curIdx = i_buffer;
 
-            // Verify space.
-            if (i_bufferSize < l_bytesWritten + 2 * l_huidSize)
+    for ( auto & entry : data )
+    {
+        // We only need the target data when the target for this entry does not
+        // match the previous entry.
+        if ( entry.chipHandle != curTrgt )
+        {
+            // Ensure we have enough space for the entry header.
+            if ( __bufferFull( i_buffer, i_bufferSize, curIdx,
+                               (sizeof(HUID) + sizeof(uint32_t)) ) )
             {
                 break;
             }
-            // Write header.
-            memcpy(&i_buffer[l_bytesWritten],
-                   &l_chipHuid, l_huidSize);
-            l_bytesWritten += l_huidSize;
-            l_entryCountPos = &i_buffer[l_bytesWritten];
-            l_ptempHandle = NULL;
-            memcpy(l_entryCountPos, &l_chipHuid, l_huidSize);
-            l_bytesWritten += l_huidSize;
+
+            // Update current target.
+            curTrgt = entry.chipHandle;
+
+            // Add HUID to buffer.
+            __bufferAdd( curIdx, htonl(PlatServices::getHuid(curTrgt)) );
+
+            // Update the current count pointer.
+            regCntPtr = (uint32_t *)curIdx;
+
+            // Zero out the register count.
+            __bufferAdd( curIdx, htonl(0) );
         }
 
-        // Go to next entry if 0 data length.
-        if (0 == i->dataByteLength)
+        // Go to next entry if the data byte length is 0.
+        if ( 0 == entry.dataByteLength )
             continue;
 
-        // Check room.
-        if ((l_bytesWritten + 2*sizeof(uint16_t) + i->dataByteLength) >
-                i_bufferSize)
-            continue;
+        // Ensure we have enough space for the entry header.
+        if ( __bufferFull( i_buffer, i_bufferSize, curIdx,
+                           (2 * sizeof(uint16_t) + entry.dataByteLength) ) )
+        {
+            break;
+        }
 
         // Write register ID.
-        uint16_t l_regId = htons(i->address);
-        memcpy(&i_buffer[l_bytesWritten], &l_regId, sizeof(l_regId));
-        l_bytesWritten += sizeof(l_regId);
-
-        // Write register length.
-        uint16_t l_regLen = htons(i->dataByteLength);
-        memcpy(&i_buffer[l_bytesWritten], &l_regLen, sizeof(l_regLen));
-        l_bytesWritten += sizeof(l_regLen);
-
-        // Write register data.
+        __bufferAdd( curIdx, htons(entry.address) );
+
+        // Write data length.
+        __bufferAdd( curIdx, htons(entry.dataByteLength) );
+
+        // Write the data.
+        // >>> TODO: RTC 199045 The data should already be in network format.
+        //           However, that is not the case. Instead, the data is
+        //           converted here, which would be is fine if we were only
+        //           adding registers, but we have additional capture data,
+        //           especially in the memory subsytem, that are actually stored
+        //           in the network format, but swizzled before adding to the
+        //           capture data. Which means we are doing too much.
+        //           Unfortunately, it currently works and will take some time
+        //           to actually do it right. Therefore, we will leave this
+        //           as-is and try to make the appropriate fix later.
         uint32_t l_dataWritten = 0;
-        while ((l_dataWritten + 4) <= i->dataByteLength)
+        while ((l_dataWritten + 4) <= entry.dataByteLength)
         {
             uint32_t l_temp32;
-            memcpy(&l_temp32, &i->dataPtr[l_dataWritten], sizeof(l_temp32));
+            memcpy(&l_temp32, &entry.dataPtr[l_dataWritten], sizeof(l_temp32));
             l_temp32 = htonl(l_temp32);
-            memcpy(&i_buffer[l_bytesWritten], &l_temp32, 4);
-            l_dataWritten += 4; l_bytesWritten += 4;
+            memcpy(curIdx, &l_temp32, 4);
+            l_dataWritten += 4; curIdx += 4;
         }
-        if (l_dataWritten != i->dataByteLength)
+        if (l_dataWritten != entry.dataByteLength)
         {
+            // TODO: RTC 199045 This is actually pretty bad because it will read
+            //       four bytes of memory, sizzle the four bytes, then write
+            //       less than four bytes to the buffer. This could cause a
+            //       buffer overrun exception if we were at the end of memory.
+            //       Also, how can we trust the right most bytes to be correct
+            //       since they technically should not be part of the entry
+            //       data? Again, we don't seem to be hitting this bug and it
+            //       will take time to fix it (see note above). Therefore, we
+            //       will leave it for now and fix it when we have time.
             uint32_t l_temp32;
-            memcpy(&l_temp32, &i->dataPtr[l_dataWritten], sizeof(l_temp32));
+            memcpy(&l_temp32, &entry.dataPtr[l_dataWritten], sizeof(l_temp32));
             l_temp32 = htonl(l_temp32);
-            memcpy(&i_buffer[l_bytesWritten],
-                   &l_temp32, i->dataByteLength - l_dataWritten);
-            l_bytesWritten += i->dataByteLength - l_dataWritten;
+            memcpy(curIdx, &l_temp32, entry.dataByteLength - l_dataWritten);
+            curIdx += entry.dataByteLength - l_dataWritten;
         }
+        // <<< TODO: RTC 199045
 
-        // Update entry count.
-        l_regEntries++;
-    }
-
-    // Update previous entry count.
-    if (NULL != l_entryCountPos)
-    {
-        l_regEntries = htonl(l_regEntries);
-        memcpy(l_entryCountPos, &l_regEntries, sizeof(l_regEntries));
-        l_regEntries = 0;
+        // Update entry count. It is important to update the buffer just in
+        // case we happen to run out of room in the buffer and need to exit
+        // early.
+        *regCntPtr = htonl( ntohl(*regCntPtr) + 1 );
     }
 
-    return l_bytesWritten;
+    return curIdx - i_buffer;
 }
 
 // dg08a -->
diff --git a/src/usr/diag/prdf/common/framework/service/xspprdsdbug.C b/src/usr/diag/prdf/common/framework/service/xspprdsdbug.C
index 855ce9bc1..47ef6afb0 100755
--- a/src/usr/diag/prdf/common/framework/service/xspprdsdbug.C
+++ b/src/usr/diag/prdf/common/framework/service/xspprdsdbug.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2012,2017                        */
+/* Contributors Listed Below - COPYRIGHT 2012,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -60,6 +60,7 @@
 #include <iipSystem.h>
 #include <prdfPluginDef.H>
 #include <algorithm>
+#include <UtilHash.H>
 
 #undef xspprdsdbug_C
 
@@ -347,7 +348,8 @@ void SYSTEM_DEBUG_CLASS::CalloutThoseAtAttention(
         AttnData ad(*i);
         BitString cbs(sizeof(AttnData)*8,(CPU_WORD *)&ad);
 
-        capture.Add(PlatServices::getSystemTarget(),0,cbs);
+        capture.Add( PlatServices::getSystemTarget(),
+                     Util::hashString("ATTN_DATA"), cbs );
     }
 
     sdc->SetCallout(LEVEL2_SUPPORT, MRU_HIGH);
diff --git a/src/usr/diag/prdf/common/plugins/prdfLogParse_common.C b/src/usr/diag/prdf/common/plugins/prdfLogParse_common.C
index 46c05c862..c6cd47d0b 100644
--- a/src/usr/diag/prdf/common/plugins/prdfLogParse_common.C
+++ b/src/usr/diag/prdf/common/plugins/prdfLogParse_common.C
@@ -5,7 +5,7 @@
 /*                                                                        */
 /* OpenPOWER HostBoot Project                                             */
 /*                                                                        */
-/* Contributors Listed Below - COPYRIGHT 2013,2018                        */
+/* Contributors Listed Below - COPYRIGHT 2013,2019                        */
 /* [+] International Business Machines Corp.                              */
 /*                                                                        */
 /*                                                                        */
@@ -158,6 +158,10 @@ void getTargetInfo( HUID i_chipId, TARGETING::TYPE & o_targetType,
 
     switch ( o_targetType )
     {
+        case TYPE_SYS:
+            snprintf( o_chipName, i_sz_chipName, "sys()" );
+            break;
+
         case TYPE_PROC:
             snprintf( o_chipName, i_sz_chipName, "pu(n%dp%d)",
                       l_node, l_chip );
@@ -364,40 +368,36 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
 
     uint32_t byteIndex = 0; // Capture Data buffer index in bytes
 
-    uint8_t * l_uncompBuffer = new uint8_t[CaptureDataSize];
-    size_t l_uncompBufSize = CaptureDataSize;
+    size_t sz_uncompBuffer = sizeof(CaptureDataClass);
+    uint8_t * uncompBuffer = new uint8_t[sz_uncompBuffer];
     CaptureDataClass * l_capData;
-    memset( l_uncompBuffer, 0xFF, CaptureDataSize );
+    memset( uncompBuffer, 0xFF, sz_uncompBuffer );
 
     if ( 2 <= i_ver ) // version 2 and above are compressed.
     {
         PrdfCompressBuffer::uncompressBuffer( ((uint8_t *) i_buffer),
                                               ((size_t) i_buflen),
-                                              l_uncompBuffer,
-                                              l_uncompBufSize );
+                                              uncompBuffer,
+                                              sz_uncompBuffer );
         //fix up the buffer length now that uncompressed
-        i_buflen = l_uncompBufSize;
-        l_capData = (CaptureDataClass *) l_uncompBuffer;
+        i_buflen = sz_uncompBuffer;
+        l_capData = (CaptureDataClass *) uncompBuffer;
     }
     else // version 1 is uncompressed.
     {
         l_capData = (CaptureDataClass *) i_buffer;
     }
 
-    // Fix endianness on size field.
-    l_capData->PfaCaptureDataSize = ntohl(l_capData->PfaCaptureDataSize);
-    if( l_capData->PfaCaptureDataSize < i_buflen )
-    {
-        i_buflen = l_capData->PfaCaptureDataSize;
-    }
+    // Get the capture data size and adjust buffer length accordingly.
+    size_t sz_capData = ntohl( l_capData->PfaCaptureDataSize );
+    if ( sz_capData < i_buflen ) i_buflen = sz_capData;
 
     i_parser.PrintBlank();
     i_parser.PrintHeading("PRD Capture Data");
     i_parser.PrintBlank();
 
     char sigHeaderString[72], sigDataString[100];
-    UtilMem lCapDataBS( l_capData->CaptureData,
-                        l_capData->PfaCaptureDataSize * 8 ); // pw06
+    UtilMem lCapDataBS( l_capData->CaptureData, sz_capData * 8 );
 
     do
     {
@@ -462,13 +462,18 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
             {
                 i_parser.PrintString( sigHeaderString, "No Data Found" );
             }
+            else if ( Util::hashString("ATTN_DATA") == sigId )
+            {
+                i_parser.PrintString( " ATTN_DEBUG", "" );
+                i_parser.PrintHexDump( sigData, sigDataSize );
+            }
             else if ( Util::hashString("MEM_UE_TABLE") == sigId )
             {
-                 parseMemUeTable( sigData, sigDataSize, i_parser );
+                parseMemUeTable( sigData, sigDataSize, i_parser );
             }
             else if ( Util::hashString("MEM_CE_TABLE") == sigId )
             {
-                 parseMemCeTable( sigData, sigDataSize, i_parser );
+                parseMemCeTable( sigData, sigDataSize, i_parser );
             }
             else if ( Util::hashString("IUE_COUNTS") == sigId )
             {
@@ -476,30 +481,30 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
             }
             else if ( Util::hashString("MEM_RCE_TABLE") == sigId )
             {
-                 parseMemRceTable( sigData, sigDataSize, i_parser );
+                parseMemRceTable( sigData, sigDataSize, i_parser );
             }
             else if ( Util::hashString("DRAM_REPAIRS_DATA") == sigId )
             {
-                 parseDramRepairsData( sigData, sigDataSize, i_parser );
+                parseDramRepairsData( sigData, sigDataSize, i_parser );
             }
             else if ( Util::hashString("DRAM_REPAIRS_VPD") == sigId )
             {
-                 parseDramRepairsVpd( sigData, sigDataSize, i_parser,
-                                      l_targetType );
+                parseDramRepairsVpd( sigData, sigDataSize, i_parser,
+                                     l_targetType );
             }
             else if ( Util::hashString("BAD_DQ_BITMAP") == sigId )
             {
-                 parseBadDqBitmap( sigData, sigDataSize, i_parser,
-                                   l_targetType );
+                parseBadDqBitmap( sigData, sigDataSize, i_parser,
+                                  l_targetType );
             }
             else if ( Util::hashString("ROW_REPAIR_VPD") == sigId )
             {
-                 parseRowRepairVpd( sigData, sigDataSize, i_parser );
+                parseRowRepairVpd( sigData, sigDataSize, i_parser );
             }
             else if ( (Util::hashString(TD_CTLR_DATA::START) == sigId) ||
                       (Util::hashString(TD_CTLR_DATA::END)   == sigId) )
             {
-                 parseTdCtlrStateData( sigData, sigDataSize, i_parser, sigId );
+                parseTdCtlrStateData( sigData, sigDataSize, i_parser, sigId );
             }
             else if ( Util::hashString("TOD_ERROR_DATA") == sigId)
             {
@@ -513,11 +518,11 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
 */
             else if ( Util::hashString(LD_CR_FFDC::L2TITLE) == sigId )
             {
-                 parseL2LdCrFfdc( sigData, sigDataSize, i_parser );
+                parseL2LdCrFfdc( sigData, sigDataSize, i_parser );
             }
             else if ( Util::hashString(LD_CR_FFDC::L3TITLE) == sigId )
             {
-                 parseL3LdCrFfdc( sigData, sigDataSize, i_parser );
+                parseL3LdCrFfdc( sigData, sigDataSize, i_parser );
             }
             else if ( (0 != sigDataSize) && (sizeof(uint64_t) >= sigDataSize) )
             {
@@ -562,7 +567,7 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
     {
         i_parser.PrintHeading("Uncompressed Capture Buffer");
         i_parser.PrintBlank();
-        i_parser.PrintHexDump( l_uncompBuffer, l_uncompBufSize );
+        i_parser.PrintHexDump( uncompBuffer, sz_uncompBuffer );
         i_parser.PrintBlank();
 
         if ( false == rc )
@@ -577,7 +582,7 @@ bool parseCaptureData( void * i_buffer, uint32_t i_buflen,
         rc = false; // force raw hex dump
     }
 
-    delete [] l_uncompBuffer;
+    delete [] uncompBuffer;
 
     return rc;
 }