From a75ee3dd41b30ae638cfc4a799c3ff1ceef0c868 Mon Sep 17 00:00:00 2001 From: jdh8d <jdh8d@git.zephyr-software.com> Date: Mon, 18 Aug 2014 14:24:09 +0000 Subject: [PATCH] Updates to make AnnotationParser work right with multiple types of annotations Former-commit-id: 7c651fb9ebc7f5374a1c823c7f999a9535cff935 --- .gitattributes | 1 + libIRDB/test/fix_calls.cpp | 5 ++- .../include/MEDS_AnnotationBase.hpp | 3 +- .../include/MEDS_AnnotationParser.hpp | 9 +++-- .../include/MEDS_FuncAnnotation.hpp | 35 +++++++++++++++++ .../include/MEDS_ProblemFuncAnnotation.hpp | 4 +- .../include/MEDS_SafeFuncAnnotation.hpp | 4 +- .../src/MEDS_AnnotationParser.cpp | 14 ++++++- .../src/MEDS_ProblemFuncAnnotation.cpp | 20 ++++++---- .../src/MEDS_SafeFuncAnnotation.cpp | 19 +++++++++- tools/ret_shadow_stack/rss_instrument.cpp | 38 ++++++++----------- 11 files changed, 109 insertions(+), 43 deletions(-) create mode 100644 libMEDSannotation/include/MEDS_FuncAnnotation.hpp diff --git a/.gitattributes b/.gitattributes index 2b34bc2db..6ca53f769 100644 --- a/.gitattributes +++ b/.gitattributes @@ -233,6 +233,7 @@ libMEDSannotation/Makefile -text libMEDSannotation/include/MEDS.hpp -text libMEDSannotation/include/MEDS_AnnotationBase.hpp -text libMEDSannotation/include/MEDS_AnnotationParser.hpp -text +libMEDSannotation/include/MEDS_FuncAnnotation.hpp -text libMEDSannotation/include/MEDS_InstructionCheckAnnotation.hpp -text libMEDSannotation/include/MEDS_ProblemFuncAnnotation.hpp -text libMEDSannotation/include/MEDS_Register.hpp -text diff --git a/libIRDB/test/fix_calls.cpp b/libIRDB/test/fix_calls.cpp index eac78e2b2..c8406a6fa 100644 --- a/libIRDB/test/fix_calls.cpp +++ b/libIRDB/test/fix_calls.cpp @@ -681,7 +681,10 @@ void fix_other_pcrel(FileIR_t* firp, Instruction_t *insn, UIntPtr virt_offset) /* put the data back into the insn */ data.replace(0, data.length(), cstr, data.length()); insn->SetDataBits(data); - insn->GetAddress()->SetVirtualOffset(0); // going to end up in the SPRI file anyhow after changing the data bits + + // going to end up in the SPRI file anyhow after changing the data bits + // and it's important to set the VO to 0, so that the pcrel-ness is calculated correctly. + insn->GetAddress()->SetVirtualOffset(0); Relocation_t *reloc=new Relocation_t; reloc->SetOffset(0); diff --git a/libMEDSannotation/include/MEDS_AnnotationBase.hpp b/libMEDSannotation/include/MEDS_AnnotationBase.hpp index 05c612d71..bbb07f481 100644 --- a/libMEDSannotation/include/MEDS_AnnotationBase.hpp +++ b/libMEDSannotation/include/MEDS_AnnotationBase.hpp @@ -30,8 +30,7 @@ class MEDS_AnnotationBase virtual void init() { m_isValid=false; } - - + virtual bool isFuncAnnotation() const { return false; } // false by default protected: diff --git a/libMEDSannotation/include/MEDS_AnnotationParser.hpp b/libMEDSannotation/include/MEDS_AnnotationParser.hpp index bb2012bad..8305b4f3c 100644 --- a/libMEDSannotation/include/MEDS_AnnotationParser.hpp +++ b/libMEDSannotation/include/MEDS_AnnotationParser.hpp @@ -13,6 +13,9 @@ namespace MEDS_Annotation typedef std::multimap<VirtualOffset, MEDS_AnnotationBase*> MEDS_Annotations_t; typedef std::pair<VirtualOffset, MEDS_AnnotationBase*> MEDS_Annotations_Pair_t; +typedef std::multimap<std::string, MEDS_AnnotationBase*> MEDS_FuncAnnotations_t; +typedef std::pair<std::string, MEDS_AnnotationBase*> MEDS_Annotations_FuncPair_t; + class MEDS_AnnotationParser { public: @@ -20,12 +23,12 @@ class MEDS_AnnotationParser MEDS_AnnotationParser(std::istream &); /* pass opened file */ void parseFile(std::istream &); void parseFile(const std::string &); /* pass filename */ - // std::multimap<VirtualOffset, MEDS_Annotation::MEDS_AnnotationBase> getAnnotations() { return m_annotations; } - MEDS_Annotations_t getAnnotations() { return m_annotations; } + MEDS_Annotations_t getAnnotations() { return m_annotations; } + MEDS_FuncAnnotations_t getFuncAnnotations() { return m_func_annotations; } private: - //std::multimap<VirtualOffset, MEDS_Annotation::MEDS_AnnotationBase> MEDS_Annotations_t m_annotations; + MEDS_FuncAnnotations_t m_func_annotations; }; } diff --git a/libMEDSannotation/include/MEDS_FuncAnnotation.hpp b/libMEDSannotation/include/MEDS_FuncAnnotation.hpp new file mode 100644 index 000000000..93be3b63c --- /dev/null +++ b/libMEDSannotation/include/MEDS_FuncAnnotation.hpp @@ -0,0 +1,35 @@ +#ifndef _MEDS_FUNCANNOTATION_H_ +#define _MEDS_FUNCANNOTATION_H_ + +#include <string> +#include "VirtualOffset.hpp" +#include "MEDS_Register.hpp" +#include "MEDS_AnnotationBase.hpp" + + +namespace MEDS_Annotation +{ + +using namespace std; +using namespace MEDS_Annotation; + +// +// Class to handle one MEDS (integer vulnerability) annotation +// +class MEDS_FuncAnnotation : public MEDS_AnnotationBase +{ + public: + MEDS_FuncAnnotation() {} + virtual ~MEDS_FuncAnnotation(){} + + virtual bool isFuncAnnotation() const { return true; } + + virtual string getFuncName() const { return m_func_name; } + virtual void setFuncName(const string &p_func_name) { m_func_name=p_func_name; } + + private: + string m_func_name; +}; + +} +#endif diff --git a/libMEDSannotation/include/MEDS_ProblemFuncAnnotation.hpp b/libMEDSannotation/include/MEDS_ProblemFuncAnnotation.hpp index d43e6f9c6..44f919cfb 100644 --- a/libMEDSannotation/include/MEDS_ProblemFuncAnnotation.hpp +++ b/libMEDSannotation/include/MEDS_ProblemFuncAnnotation.hpp @@ -4,7 +4,7 @@ #include <string> #include "VirtualOffset.hpp" #include "MEDS_Register.hpp" -#include "MEDS_AnnotationBase.hpp" +#include "MEDS_FuncAnnotation.hpp" namespace MEDS_Annotation @@ -19,7 +19,7 @@ using namespace MEDS_Annotation; // // Class to handle one MEDS (integer vulnerability) annotation // -class MEDS_ProblemFuncAnnotation : public MEDS_AnnotationBase +class MEDS_ProblemFuncAnnotation : public MEDS_FuncAnnotation { public: MEDS_ProblemFuncAnnotation() {}; diff --git a/libMEDSannotation/include/MEDS_SafeFuncAnnotation.hpp b/libMEDSannotation/include/MEDS_SafeFuncAnnotation.hpp index 47653b8d5..ff54371e7 100644 --- a/libMEDSannotation/include/MEDS_SafeFuncAnnotation.hpp +++ b/libMEDSannotation/include/MEDS_SafeFuncAnnotation.hpp @@ -4,7 +4,7 @@ #include <string> #include "VirtualOffset.hpp" #include "MEDS_Register.hpp" -#include "MEDS_AnnotationBase.hpp" +#include "MEDS_FuncAnnotation.hpp" namespace MEDS_Annotation @@ -19,7 +19,7 @@ using namespace MEDS_Annotation; // // Class to handle one MEDS (integer vulnerability) annotation // -class MEDS_SafeFuncAnnotation : public MEDS_AnnotationBase +class MEDS_SafeFuncAnnotation : public MEDS_FuncAnnotation { public: MEDS_SafeFuncAnnotation() {}; diff --git a/libMEDSannotation/src/MEDS_AnnotationParser.cpp b/libMEDSannotation/src/MEDS_AnnotationParser.cpp index 10b7e0e36..f9cea504a 100644 --- a/libMEDSannotation/src/MEDS_AnnotationParser.cpp +++ b/libMEDSannotation/src/MEDS_AnnotationParser.cpp @@ -41,8 +41,18 @@ void MEDS_AnnotationParser::parseFile(istream &p_inputStream) type * annot=new type(line); \ if (annot->isValid()) \ { \ - VirtualOffset vo = annot->getVirtualOffset(); \ - m_annotations.insert(MEDS_Annotations_Pair_t(vo, annot)); \ + if(annot->isFuncAnnotation()) \ + { \ + MEDS_FuncAnnotation* fannot=dynamic_cast<MEDS_FuncAnnotation*>(annot); \ + assert(fannot); \ + string nam=fannot->getFuncName(); \ + m_func_annotations.insert(MEDS_Annotations_FuncPair_t(nam, annot)); \ + } \ + else \ + { \ + VirtualOffset vo = annot->getVirtualOffset(); \ + m_annotations.insert(MEDS_Annotations_Pair_t(vo, annot)); \ + } \ continue; \ } \ else \ diff --git a/libMEDSannotation/src/MEDS_ProblemFuncAnnotation.cpp b/libMEDSannotation/src/MEDS_ProblemFuncAnnotation.cpp index dec8bb823..57ede5a57 100644 --- a/libMEDSannotation/src/MEDS_ProblemFuncAnnotation.cpp +++ b/libMEDSannotation/src/MEDS_ProblemFuncAnnotation.cpp @@ -1,5 +1,5 @@ #include <stdlib.h> - +#include <string.h> #include <iostream> #include <cstdio> #include <string> @@ -23,7 +23,7 @@ MEDS_ProblemFuncAnnotation::MEDS_ProblemFuncAnnotation(const string &p_rawLine) void MEDS_ProblemFuncAnnotation::init() { - MEDS_AnnotationBase::init(); + MEDS_FuncAnnotation::init(); } @@ -42,27 +42,33 @@ void MEDS_ProblemFuncAnnotation::parse() if (m_rawInputLine.find("FUNC ")==string::npos) return; - if (m_rawInputLine.find("FUNC PROBLEM")==string::npos ) + size_t pos=m_rawInputLine.find("FUNC PROBLEM"); + if ( pos==string::npos ) { /* FUNC line that's not local or global? I'm confused. */ /* could be a FUNC GLOBAL, etc. line */ return; } + size_t func_name_start_pos=pos+strlen("FUNC PROBLEM "); + size_t func_end_pos=m_rawInputLine.find(" ", func_name_start_pos); + assert(func_end_pos!=string::npos); + string func_name=m_rawInputLine.substr(func_name_start_pos, func_end_pos-func_name_start_pos); + // get offset - VirtualOffset vo(m_rawInputLine); - m_virtualOffset = vo; + setFuncName(func_name); + cout<<"Found problem func name='"<<func_name<<"'"<<endl; if (m_rawInputLine.find("JUMPUNRESOLVED")!=string::npos) { if(getenv("VERBOSE")) - cout<<"Found JUMPUNRESOLVED problem annotation for "<<vo.to_string()<<endl; + cout<<"Found JUMPUNRESOLVED problem annotation for "<<getFuncName() << endl; markJumpUnresolved(); } else if (m_rawInputLine.find("CALLUNRESOLVED")!=string::npos) { if(getenv("VERBOSE")) - cout<<"Found CALLUNRESOLVED annotation for "<<vo.to_string()<<endl; + cout<<"Found CALLUNRESOLVED annotation for "<<getFuncName() << endl; markCallUnresolved(); } diff --git a/libMEDSannotation/src/MEDS_SafeFuncAnnotation.cpp b/libMEDSannotation/src/MEDS_SafeFuncAnnotation.cpp index 380165a81..b4d7ec1d0 100644 --- a/libMEDSannotation/src/MEDS_SafeFuncAnnotation.cpp +++ b/libMEDSannotation/src/MEDS_SafeFuncAnnotation.cpp @@ -3,6 +3,7 @@ #include <iostream> #include <cstdio> #include <string> +#include <string.h> #include "MEDS_Register.hpp" #include "MEDS_SafeFuncAnnotation.hpp" @@ -23,7 +24,7 @@ MEDS_SafeFuncAnnotation::MEDS_SafeFuncAnnotation(const string &p_rawLine) void MEDS_SafeFuncAnnotation::init() { - MEDS_AnnotationBase::init(); + MEDS_FuncAnnotation::init(); } @@ -54,6 +55,22 @@ void MEDS_SafeFuncAnnotation::parse() VirtualOffset vo(m_rawInputLine); m_virtualOffset = vo; + size_t func_name_start_pos=0, pos=0; + if ((pos=m_rawInputLine.find("FUNC GLOBAL"))!=string::npos) + func_name_start_pos=pos+strlen("FUNC GLOBAL "); + else if ((pos=m_rawInputLine.find("FUNC LOCAL"))!=string::npos) + func_name_start_pos=pos+strlen("FUNC LOCAL "); + + size_t func_end_pos=m_rawInputLine.find(" ", func_name_start_pos); + assert(func_end_pos!=string::npos); + string func_name=m_rawInputLine.substr(func_name_start_pos, func_end_pos-func_name_start_pos); + + cout<<"Found safe func name='"<<func_name<<"'"<<endl; + + // get offset + setFuncName(func_name); + + if (m_rawInputLine.find(" FUNC_SAFE ")!=string::npos) { if(getenv("VERBOSE")) diff --git a/tools/ret_shadow_stack/rss_instrument.cpp b/tools/ret_shadow_stack/rss_instrument.cpp index 36fbf5c2c..cb8b9b6e0 100644 --- a/tools/ret_shadow_stack/rss_instrument.cpp +++ b/tools/ret_shadow_stack/rss_instrument.cpp @@ -340,24 +340,15 @@ static bool is_safe_func(Function_t* func, MEDS_AnnotationParser* meds_ap) if(!func->GetEntryPoint()) return false; - /* grab vo from IRDB */ - virtual_offset_t irdb_vo = func->GetEntryPoint()->GetAddress()->GetVirtualOffset(); - - /* no original address for this function? already instrumented and getting data from MEDS is a bad idea? */ - if (irdb_vo == 0) - return false; - - VirtualOffset vo(irdb_vo); - - std::pair<MEDS_Annotations_t::iterator,MEDS_Annotations_t::iterator> ret; + std::pair<MEDS_FuncAnnotations_t::iterator,MEDS_FuncAnnotations_t::iterator> ret; /* find it in the annotations */ - ret = meds_ap->getAnnotations().equal_range(vo); + ret = meds_ap->getFuncAnnotations().equal_range(func->GetName()); MEDS_SafeFuncAnnotation annotation; MEDS_SafeFuncAnnotation* p_annotation; /* for each annotation for this instruction */ - for (MEDS_Annotations_t::iterator it = ret.first; it != ret.second; ++it) + for (MEDS_FuncAnnotations_t::iterator it = ret.first; it != ret.second; ++it) { /* is this annotation a funcSafe annotation? */ p_annotation=dynamic_cast<MEDS_SafeFuncAnnotation*>(it->second); @@ -385,24 +376,16 @@ static bool is_problem_func(Function_t* func, MEDS_AnnotationParser* meds_ap) if(!func->GetEntryPoint()) return false; - /* grab vo from IRDB */ - virtual_offset_t irdb_vo = func->GetEntryPoint()->GetAddress()->GetVirtualOffset(); - /* no original address for this function? already instrumented and getting data from MEDS is a bad idea? */ - if (irdb_vo == 0) - return false; - - VirtualOffset vo(irdb_vo); - - std::pair<MEDS_Annotations_t::iterator,MEDS_Annotations_t::iterator> ret; + std::pair<MEDS_FuncAnnotations_t::iterator,MEDS_FuncAnnotations_t::iterator> ret; /* find it in the annotations */ - ret = meds_ap->getAnnotations().equal_range(vo); + ret = meds_ap->getFuncAnnotations().equal_range(func->GetName()); MEDS_ProblemFuncAnnotation annotation; MEDS_ProblemFuncAnnotation* p_annotation; /* for each annotation for this instruction */ - for (MEDS_Annotations_t::iterator it = ret.first; it != ret.second; ++it) + for (MEDS_FuncAnnotations_t::iterator it = ret.first; it != ret.second; ++it) { /* is this annotation a funcSafe annotation? */ p_annotation=dynamic_cast<MEDS_ProblemFuncAnnotation*>(it->second); @@ -423,20 +406,25 @@ static bool is_problem_func(Function_t* func, MEDS_AnnotationParser* meds_ap) } +static int safe_funcs=0,problem_funcs=0, instr_funcs=0; + static bool needs_rss_instrumentation(Function_t* func, MEDS_AnnotationParser* meds_ap) { if(is_safe_func(func,meds_ap)) { + safe_funcs++; return false; // safe functions need no instrumentation } if(is_problem_func(func,meds_ap)) { + problem_funcs++; return false; // problem funcs can't have instrumentation } /* otherwise, we need to instrument */ + instr_funcs++; return true; } @@ -467,6 +455,10 @@ bool RSS_Instrument::execute() } + cout << "# ATTRIBUTE safe_funcs=" <<std::dec<<safe_funcs<<endl; + cout << "# ATTRIBUTE problem_funcs=" <<problem_funcs<<endl; + cout << "# ATTRIBUTE instr_funcs=" <<instr_funcs<<endl; + /* return an exit code */ if(success) return 0; /* success? */ -- GitLab