Jump tables should be only in read only sections

dyninst · Jun 10, 2016 · 4e5f3ce · 4e5f3ce
1 parent 1c10fba
commit 4e5f3ce
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 4 deletions.
diff --git a/dyninstAPI/src/addressSpace.C b/dyninstAPI/src/addressSpace.C
@@ -844,6 +844,10 @@ bool AddressSpace::isData(const Address addr) const {
    return false;
 }
 
+bool AddressSpace::isReadOnly(const Address ) const {
+   return false;
+}
+
 bool AddressSpace::isValidAddress(const Address addr) const {
    mapped_object *obj = findObject(addr);
    if (!obj) return false;

diff --git a/dyninstAPI/src/addressSpace.h b/dyninstAPI/src/addressSpace.h
@@ -196,6 +196,7 @@ class AddressSpace : public InstructionSource {
     bool usesDataLoadAddress() const; // OS-specific
     virtual bool isCode(const Address) const;
     virtual bool isData(const Address) const;
+    virtual bool isReadOnly(const Address) const;
     virtual Address offset() const = 0;
     virtual Address length() const = 0;
     virtual Architecture getArch() const = 0;

diff --git a/parseAPI/h/CFGModifier.h b/parseAPI/h/CFGModifier.h
@@ -113,6 +113,7 @@ class InsertedRegion : public CodeRegion {
    };
    PARSER_EXPORT bool isCode(const Address a) const { return isValidAddress(a); };
    PARSER_EXPORT bool isData(const Address) const { return false; };
+   PARSER_EXPORT bool isReadOnly(const Address) const { return false; };
    PARSER_EXPORT Address offset() const { return base_; };
    PARSER_EXPORT Address length() const { return size_; };
    PARSER_EXPORT unsigned int getAddressWidth() const {

diff --git a/parseAPI/h/CodeSource.h b/parseAPI/h/CodeSource.h
@@ -230,6 +230,7 @@ class PARSER_EXPORT SymtabCodeRegion : public CodeRegion {
     unsigned int getAddressWidth() const;
     bool isCode(const Address) const;
     bool isData(const Address) const;
+    bool isReadOnly(const Address) const;
     Address offset() const;
     Address length() const;
     Architecture getArch() const;
@@ -282,6 +283,7 @@ class PARSER_EXPORT SymtabCodeSource : public CodeSource {
     unsigned int getAddressWidth() const;
     bool isCode(const Address) const;
     bool isData(const Address) const;
+    bool isReadOnly(const Address) const;
     Address offset() const;
     Address length() const;
     Architecture getArch() const;

diff --git a/parseAPI/h/InstructionSource.h b/parseAPI/h/InstructionSource.h
@@ -49,6 +49,7 @@ class PARSER_EXPORT InstructionSource
     virtual unsigned int getAddressWidth() const = 0;
     virtual bool isCode(const Address) const = 0;
     virtual bool isData(const Address) const = 0;
+    virtual bool isReadOnly(const Address) const = 0;
     virtual Address offset() const = 0;
     virtual Address length() const = 0;
     virtual Architecture getArch() const = 0;

diff --git a/parseAPI/src/BoundFactData.C b/parseAPI/src/BoundFactData.C
@@ -635,12 +635,16 @@ void BoundValue::MemoryRead(Block* b, int readSize) {
 #endif
 		if (IsInReadOnlyRegion(memAddrLow, memAddrHigh)) {
 		    set<uint64_t> values;
-                    if (interval.size() <= MAX_TABLE_ENTRY && b->obj()->cs()->isValidAddress(memAddrLow)) {
+		    if (interval.size() <= MAX_TABLE_ENTRY && b->obj()->cs()->isValidAddress(memAddrLow) && b->obj()->cs()->isReadOnly(memAddrLow)) {
 		        for (Address memAddr = memAddrLow ; memAddr <= memAddrHigh; memAddr += interval.stride) {
 			    if (!b->obj()->cs()->isValidAddress(memAddr)) {
 			        parsing_printf("INVALID ADDRESS %lx\n", memAddr);
 			        continue;			
 			    }
+                            if (!b->obj()->cs()->isReadOnly(memAddr)) {
+                                parsing_printf("NOT READ ONLY SECTION %lx\n", memAddr);
+                                continue;                       
+                            }
 			    uint64_t val;
 			    switch (readSize) {
 			        case 8:

diff --git a/parseAPI/src/IndirectASTVisitor.C b/parseAPI/src/IndirectASTVisitor.C
@@ -407,6 +407,11 @@ AST::Ptr JumpTableFormatVisitor::visit(DataflowAPI::RoseAST *ast) {
 		        parsing_printf("\ttableBase 0x%lx invalid, not jump table format\n", tableBase);
 			findIncorrectFormat = true;
 		    }
+                    if (!b->obj()->cs()->isReadOnly(tableBase)) {
+		        parsing_printf("\ttableBase 0x%lx not read only, not jump table format\n", tableBase);
+			findIncorrectFormat = true;
+		    }
+
 		}
 	    }
 	}

diff --git a/parseAPI/src/IndirectAnalyzer.C b/parseAPI/src/IndirectAnalyzer.C
@@ -17,7 +17,7 @@ using namespace Dyninst::InstructionAPI;
 
 
 bool IndirectControlFlowAnalyzer::NewJumpTableAnalysis(std::vector<std::pair< Address, Dyninst::ParseAPI::EdgeTypeEnum > >& outEdges) {
-    if (block->last() == 0xa2219d) dyn_debug_parsing = 1; else dyn_debug_parsing=0;
+//    if (block->last() == 0xacef04) dyn_debug_parsing = 1; else dyn_debug_parsing=0;
     parsing_printf("Apply indirect control flow analysis at %lx\n", block->last());
     parsing_printf("Looking for thunk\n");
 //  Find all blocks that reach the block containing the indirect jump
@@ -56,7 +56,7 @@ bool IndirectControlFlowAnalyzer::NewJumpTableAnalysis(std::vector<std::pair< Ad
 	bool ijt = jtp.IsJumpTable(g, bfc, target);
 	if (ijt) jtp.FillInOutEdges(target, jumpTableOutEdges);
     }
-    fprintf(stderr, "indirect jump at %lx with %d assignments and %d edges\n", block->last(), jtp.currentAssigns.size(), jumpTableOutEdges.size()); 
+    //fprintf(stderr, "indirect jump at %lx with %d assignments and %d edges\n", block->last(), jtp.currentAssigns.size(), jumpTableOutEdges.size()); 
 
     outEdges.insert(outEdges.end(), jumpTableOutEdges.begin(), jumpTableOutEdges.end());
     return !jumpTableOutEdges.empty();

diff --git a/parseAPI/src/JumpTablePred.C b/parseAPI/src/JumpTablePred.C
@@ -268,12 +268,20 @@ bool JumpTablePred::FillInOutEdges(BoundValue &target,
     if (!block->obj()->cs()->isValidAddress(tableBase)) {
         parsing_printf("\ttableBase 0x%lx invalid, returning false\n", tableBase);
 	jumpTableFormat = false;
-	fprintf(stderr, "Not jump table format!\n");
+	parsing_printf("Not jump table format!\n");
 	return false;
     }
+    if (!block->obj()->cs()->isReadOnly(tableBase)) {
+        parsing_printf("\ttableBase 0x%lx not read only, returning false\n", tableBase);
+	jumpTableFormat = false;
+	parsing_printf("Not jump table format!\n");
+        return false;
+    }
+
 
     for (Address tableEntry = tableBase; tableEntry <= tableLastEntry; tableEntry += target.interval.stride) {
 	if (!block->obj()->cs()->isValidAddress(tableEntry)) continue;
+	if (!block->obj()->cs()->isReadOnly(tableEntry)) continue;
 	Address targetAddress = 0;
 	if (target.tableReadSize > 0) {
 	    switch (target.tableReadSize) {

diff --git a/parseAPI/src/SymtabCodeSource.C b/parseAPI/src/SymtabCodeSource.C
@@ -191,6 +191,16 @@ SymtabCodeRegion::isData(const Address addr) const
            _region->getRegionType()==SymtabAPI::Region::RT_TEXTDATA;
 }
 
+bool
+SymtabCodeRegion::isReadOnly(const Address addr) const
+{
+    if(!contains(addr)) return false;
+    if (_region->getRegionName() == ".data.rel.ro") return true;
+//    parsing_printf("Region name %s, permission %d\n", _region->getRegionName().c_str(), _region->getRegionPermissions());
+    return _region->getRegionPermissions() == SymtabAPI::Region::RP_R ||
+           _region->getRegionPermissions() == SymtabAPI::Region::RP_RX;
+}
+
 Address
 SymtabCodeRegion::offset() const
 {
@@ -685,6 +695,19 @@ SymtabCodeSource::isData(const Address addr) const
         return false;
 }
 
+bool
+SymtabCodeSource::isReadOnly(const Address addr) const
+{
+    overlapping_warn(FILE__,__LINE__);
+
+    CodeRegion * cr = lookup_region(addr);
+    if(cr)
+        return cr->isReadOnly(addr);
+    else
+        return false;
+}
+
+
 Address
 SymtabCodeSource::offset() const
 {