ruby: fixes functional writes to RubyRequest

The functional write code was assuming that all writes are block sized,
which may not be true for Ruby Requests. This bug can lead to a buffer
overflow.

Committed by: Nilay Vaish <nilay@cs.wisc.edu>

diff --git a/src/mem/ruby/slicc_interface/RubyRequest.cc b/src/mem/ruby/slicc_interface/RubyRequest.cc
index 7ff2b75d839b2b3b8568ec28fead4c19235ab57b..ca0ab059f07abb0795c32e42ebfab0c0c68ff716 100644 (file)
--- a/src/mem/ruby/slicc_interface/RubyRequest.cc
+++ b/src/mem/ruby/slicc_interface/RubyRequest.cc
@@ -39,19 +39,19 @@ RubyRequest::functionalWrite(Packet *pkt)
     // has to overwrite the data for the timing request, even if the
     // timing request has still not been ordered globally.
 
-    Address pktLineAddr(pkt->getAddr());
-    pktLineAddr.makeLineAddress();
+    Addr wBase = pkt->getAddr();
+    Addr wTail = wBase + pkt->getSize();
+    Addr mBase = m_PhysicalAddress.getAddress();
+    Addr mTail = mBase + m_Size;
 
-    if (pktLineAddr == m_LineAddress) {
-        uint8_t *pktData = pkt->getPtr<uint8_t>(true);
-        unsigned int size_in_bytes = pkt->getSize();
-        unsigned startByte = pkt->getAddr() - m_LineAddress.getAddress();
+    uint8_t * pktData = pkt->getPtr<uint8_t>(true);
 
-        for (unsigned i = 0; i < size_in_bytes; ++i) {
-            data[i + startByte] = pktData[i];
-        }
+    Addr cBase = std::max(wBase, mBase);
+    Addr cTail = std::min(wTail, mTail);
 
-        return true;
+    for (Addr i = cBase; i < cTail; ++i) {
+        data[i - mBase] = pktData[i - wBase];
     }
-    return false;
+
+    return cBase < cTail;
 }

diff --git a/src/mem/ruby/system/System.cc b/src/mem/ruby/system/System.cc
index f1a6a91b86379add47d07a9a4717249033e0be8a..0e52df2ecec8eb678b285c49bd18344799fa9cba 100644 (file)
--- a/src/mem/ruby/system/System.cc
+++ b/src/mem/ruby/system/System.cc
@@ -536,13 +536,18 @@ RubySystem::functionalWrite(PacketPtr pkt)
     unsigned int size_in_bytes = pkt->getSize();
     unsigned startByte = addr.getAddress() - line_addr.getAddress();
 
+    uint32_t M5_VAR_USED num_functional_writes = 0;
+
     for (unsigned int i = 0; i < num_controllers;++i) {
-        m_abs_cntrl_vec[i]->functionalWriteBuffers(pkt);
+        num_functional_writes +=
+            m_abs_cntrl_vec[i]->functionalWriteBuffers(pkt);
 
         access_perm = m_abs_cntrl_vec[i]->getAccessPermission(line_addr);
         if (access_perm != AccessPermission_Invalid &&
             access_perm != AccessPermission_NotPresent) {
 
+            num_functional_writes++;
+
             DataBlock& block = m_abs_cntrl_vec[i]->getDataBlock(line_addr);
             DPRINTF(RubySystem, "%s\n",block);
             for (unsigned j = 0; j < size_in_bytes; ++j) {
@@ -552,7 +557,6 @@ RubySystem::functionalWrite(PacketPtr pkt)
         }
     }
 
-    uint32_t M5_VAR_USED num_functional_writes = 0;
     for (unsigned int i = 0; i < m_memory_controller_vec.size() ;++i) {
         num_functional_writes +=
             m_memory_controller_vec[i]->functionalWriteBuffers(pkt);