changeset f7d17d8a854c in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=f7d17d8a854c
description:
        arm: Fix broken page table permissions checks in remote GDB

        The remote GDB interface currently doesn't check if translations are
        valid before reading memory. This causes a panic when GDB tries to
        access unmapped memory (e.g., when getting a stack trace). There are
        two reasons for this: 1) The function used to check for valid
        translations (virtvalid()) doesn't work and panics on invalid
        translations. 2) The method in the GDB interface used to test if a
        translation is valid (RemoteGDB::acc) always returns true regardless
        of the return from virtvalid().

        This changeset fixes both of these issues.

diffstat:

 src/arch/arm/remote_gdb.cc |  15 ++++++---------
 src/arch/arm/vtophys.cc    |  27 +++++++++++++++++++--------
 2 files changed, 25 insertions(+), 17 deletions(-)

diffs (87 lines):

diff -r 4206946d60fe -r f7d17d8a854c src/arch/arm/remote_gdb.cc
--- a/src/arch/arm/remote_gdb.cc        Thu Feb 26 09:58:26 2015 -0600
+++ b/src/arch/arm/remote_gdb.cc        Mon Mar 02 04:00:27 2015 -0500
@@ -142,6 +142,7 @@
 #include "arch/arm/system.hh"
 #include "arch/arm/utility.hh"
 #include "arch/arm/vtophys.hh"
+#include "base/chunk_generator.hh"
 #include "base/intmath.hh"
 #include "base/remote_gdb.hh"
 #include "base/socket.hh"
@@ -172,16 +173,12 @@
 RemoteGDB::acc(Addr va, size_t len)
 {
     if (FullSystem) {
-        Addr last_va;
-        va       = truncPage(va);
-        last_va  = roundPage(va + len);
-
-        do  {
-            if (virtvalid(context, va)) {
-                return true;
+        for (ChunkGenerator gen(va, len, PageBytes); !gen.done(); gen.next()) {
+            if (!virtvalid(context, gen.addr())) {
+                DPRINTF(GDBAcc, "acc:   %#x mapping is invalid\n", va);
+                return false;
             }
-            va += PageBytes;
-        } while (va < last_va);
+        }
 
         DPRINTF(GDBAcc, "acc:   %#x mapping is valid\n", va);
         return true;
diff -r 4206946d60fe -r f7d17d8a854c src/arch/arm/vtophys.cc
--- a/src/arch/arm/vtophys.cc   Thu Feb 26 09:58:26 2015 -0600
+++ b/src/arch/arm/vtophys.cc   Mon Mar 02 04:00:27 2015 -0500
@@ -63,8 +63,8 @@
     fatal("VTOPHYS: Can't convert vaddr to paddr on ARM without a thread 
context");
 }
 
-Addr
-ArmISA::vtophys(ThreadContext *tc, Addr addr)
+static std::pair<bool, Addr>
+try_translate(ThreadContext *tc, Addr addr)
 {
     Fault fault;
     // Set up a functional memory Request to pass to the TLB
@@ -82,22 +82,33 @@
     tlb = static_cast<ArmISA::TLB*>(tc->getDTBPtr());
     fault = tlb->translateFunctional(&req, tc, BaseTLB::Read, TLB::NormalTran);
     if (fault == NoFault)
-        return req.getPaddr();
+        return std::make_pair(true, req.getPaddr());
 
     tlb = static_cast<ArmISA::TLB*>(tc->getITBPtr());
     fault = tlb->translateFunctional(&req, tc, BaseTLB::Read, TLB::NormalTran);
     if (fault == NoFault)
-        return req.getPaddr();
+        return std::make_pair(true, req.getPaddr());
 
-    panic("Table walkers support functional accesses. We should never get 
here\n");
+    return std::make_pair(false, 0);
+}
+
+Addr
+ArmISA::vtophys(ThreadContext *tc, Addr addr)
+{
+    const std::pair<bool, Addr> translation(try_translate(tc, addr));
+
+    if (translation.first)
+        return translation.second;
+    else
+        panic("Table walkers support functional accesses. We should never get 
here\n");
 }
 
 bool
 ArmISA::virtvalid(ThreadContext *tc, Addr vaddr)
 {
-    if (vtophys(tc, vaddr) != -1)
-        return true;
-    return false;
+    const std::pair<bool, Addr> translation(try_translate(tc, vaddr));
+
+    return translation.first;
 }
 
 
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to