diff options
author | Andreas Sandberg <Andreas.Sandberg@ARM.com> | 2015-03-02 04:00:27 -0500 |
---|---|---|
committer | Andreas Sandberg <Andreas.Sandberg@ARM.com> | 2015-03-02 04:00:27 -0500 |
commit | 34dcd90b61b6fcde7f4ff1cd717f71edca40f9b9 (patch) | |
tree | a94eaca5591e9f6027e6d279b2ee004d098d38e9 /src/arch/arm | |
parent | 670f44e05eb8eb1a56b36c4390cf83807a28d823 (diff) | |
download | gem5-34dcd90b61b6fcde7f4ff1cd717f71edca40f9b9.tar.xz |
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 (limited to 'src/arch/arm')
-rw-r--r-- | src/arch/arm/remote_gdb.cc | 15 | ||||
-rw-r--r-- | src/arch/arm/vtophys.cc | 27 |
2 files changed, 25 insertions, 17 deletions
diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc index 1686cab39..d52a9db17 100644 --- a/src/arch/arm/remote_gdb.cc +++ b/src/arch/arm/remote_gdb.cc @@ -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 @@ bool 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 --git a/src/arch/arm/vtophys.cc b/src/arch/arm/vtophys.cc index bed76acbd..3aad35818 100644 --- a/src/arch/arm/vtophys.cc +++ b/src/arch/arm/vtophys.cc @@ -63,8 +63,8 @@ ArmISA::vtophys(Addr vaddr) 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 @@ ArmISA::vtophys(ThreadContext *tc, Addr addr) 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; } |