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>

3 files changed, 110 insertions, 85 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);