Make unicorn be pull-based #97

yan · 2017-03-22T19:01:57Z

No description provided.

feliam

Spectacularly awesome, true hero power!
I still need to review the _to_raise / _should_try_again logic.

feliam · 2017-03-27T22:05:02Z

manticore/core/cpu/abstractcpu.py

        try:
            implementation = getattr(self, name)
        except AttributeError as ae:
-            logger.debug("UNIMPLEMENTED INSTRUCTION: 0x%016x:\t%s\t%s\t%s", instruction.address, ' '.join(map(lambda x: '%02x'%x, instruction.bytes)), instruction.mnemonic, instruction.op_str)
+            # Make sure we're referencing the instruction look up
+            if name not in ae.message:


Or...
if hasattr(self.name): raise

Do you have any strong thoughts on this? One argument for the current situation is not having an instruction is an exceptional situation, not normal control flow.

class B(object):
def two(self):
return 2

class A(object):
def one(self):
b = B()
return b.one()

a = A()
try :
a.one()
except Exception,e:
print "'one' in e:", 'one' in e.message
print e

I meant :

if hasattr(self.name):

instead of:

if name not in ae.message:

feliam · 2017-03-27T22:10:24Z

manticore/core/memory.py

@@ -403,6 +403,9 @@ def page_mask(self):
    def maps(self):
        return self._maps

+    def map_of_address(self, address):


Not used?
$ grep map_of_address . -Rn
./manticore/core/memory.py:406: def map_of_address(self, address):

feliam · 2017-03-27T22:50:49Z

manticore/utils/emulate.py

+        self._emu = self._unicorn()
+
+        # Copy in the concrete values of all needed registers.
+        registers = set(self._cpu.canonical_registers)


We need to at least document that ...
Ideally here we should only concretize registers that are going to be used/read. Otherwise we may destroy a lot of symbolic state that we don't need to.

There is some support for this in Capstone.instruction..regs_access()

Attempted here:
https://github.com/trailofbits/manticore/blob/dev-fix-unicorn/manticore/core/cpu/abstractcpu.py#L416-L432

feliam · 2017-03-27T22:53:38Z

manticore/utils/emulate.py

+            val = self._cpu.read_register(reg)
+            if issymbolic(val):
+                from ..core.cpu.abstractcpu import ConcretizeRegister
+                raise ConcretizeRegister(reg, "Concretizing register for emulation.") 


Lookout this will use default concretization policy (I think MINMAX?).
10 symbolic regs will spawn... a lot of states?
We should consider adding a policy='ONE' here.
Policy 'ONE' does not exist.

feliam · 2017-03-27T23:01:56Z

manticore/core/cpu/arm.py

@@ -277,26 +277,26 @@ def __setstate__(self, state):
        self._last_flags = state['_last_flags']
        self._force_next = state['_force_next']

-    def _concretize_registers(cpu, instruction):
+    def _concretize_registers(self, instruction):


Is this used?

Not used, was left from your code. Okay to remove and then re-add once we capstone adds the functionality?

yes remove everywhere and lets re implement it when needed, if needed

feliam · 2017-03-28T18:30:48Z

manticore/core/cpu/abstractcpu.py

@@ -544,18 +454,21 @@ class Syscall(CpuInterrupt):
    def __init__(self):
        super(Syscall, self).__init__("CPU Syscall")

+# TODO(yan): Move this into State or a more appropriate location
+_ValidPolicies = ['MINMAX', 'ALL', 'SAMPLED', 'ONE']


This makes it sound like we need a base Concretize(Exception).

offlinemark · 2017-03-28T21:30:45Z

manticore/utils/emulate.py

+                # XXX(yan): We might need to raise here
+                pass
+
+        if logger.getEffectiveLevel() == logging.DEBUG:


logger.isEnabledFor() might be the preferred api here https://docs.python.org/2/howto/logging.html#optimization

feliam · 2017-03-28T21:54:33Z

manticore/utils/emulate.py

+                    uc.mem_map(address & ~0xFFF, 0x1000)
+
+                    from ..core.cpu.abstractcpu import InvalidPCException
+                    self._to_raise = InvalidPCException(address)


How do we know we are trying to execute from address?
Also normally provide the mapping for the current single instruction executed.

That mapping gets added/memory brought in on line 191. And the offending address is a param to the hook

How can it be the case we are executing on not mapped memory if we provide a map for the initial emulation and we emulate only 1 instruction?

Also how do you discern READ from EXECUTE ? Can't that access be a buggy operand trying to access those arm special addresses ?

feliam · 2017-03-28T21:55:38Z

manticore/utils/emulate.py

+                    from ..core.cpu.abstractcpu import InvalidPCException
+                    self._to_raise = InvalidPCException(address)
+                else:
+                    self._to_raise = MemoryException("Can't read at", address)


Shouldn't X86 et.al also raise an exception here? (I may be seeing the indentation wrong)

It will, just the line above is a bit redundant. MemoryException will be raised on line 220 if we return False from here. I'll remove the above line during next cleanup.

feliam · 2017-03-28T21:58:06Z

manticore/utils/emulate.py

+            self._should_try_again = False
+            return False
+
+        # XXX(yan): handle if this points to an incorrect mapping


Ah no. Here is where we should check the mcore maps and raise if not mapped there.

feliam · 2017-04-03T14:33:06Z

manticore/utils/emulate.py

+
+        try:
+            m = self._create_emulated_mapping(uc, address, size)
+        except:


Shouldn't we filter for specific exceptions here?

Good catch (updating now)

offlinemark · 2017-04-03T19:08:26Z

manticore/models/linux.py

+                ).decode('hex')
+
+        # Map a TLS segment
+        self._arm_tls_memory = cpu.memory.mmap(None, 4, 'rw ')


it's not only tls in this memory, it's "kernel user helper memory" technically, so i think a rename would make it more accurate

It is only TLS memory though. What else can be in it?

sorry, my mistake, should have read more closely :(

offlinemark · 2017-04-03T19:26:37Z

manticore/utils/emulate.py

+
+    def _step(self, instruction):
+        '''
+        A single attempt at execution an instruction.


offlinemark · 2017-04-03T19:32:22Z

manticore/utils/emulate.py

+        if saved_PC == mu_pc:
+            self._cpu.PC = saved_PC + instruction.size
+
+        # Raise the exception from a hook that Uniocrn would have eaten


feliam · 2017-04-03T19:36:33Z

manticore/utils/emulate.py

+
+        #Unicorn hack. On single step unicorn wont advance the PC register
+        mu_pc = self.get_unicorn_pc()
+        if saved_PC == mu_pc:


A jump to self may be a problem here (if Unicorn actually wont update the pc for that one)

yan force-pushed the dev-fix-unicorn-hax branch from 5fc8ec6 to dadeb66 Compare March 22, 2017 19:18

offlinemark force-pushed the dev-fix-unicorn-hax branch from dadeb66 to 4451c56 Compare March 22, 2017 20:49

yan force-pushed the dev-fix-unicorn-hax branch 2 times, most recently from fccc3f8 to 0e13c9c Compare March 27, 2017 13:54

feliam reviewed Mar 27, 2017

View reviewed changes

feliam reviewed Mar 28, 2017

View reviewed changes

offlinemark reviewed Mar 28, 2017

View reviewed changes

feliam reviewed Mar 28, 2017

View reviewed changes

feliam reviewed Apr 3, 2017

View reviewed changes

feliam and others added 19 commits April 3, 2017 14:44

Enable simple ARM register concretization for Unicorn

c7c0eef

Check for regs_access better

7beb7c4

Add canonical_registers property to abstractcpu

f22b12e

cpu to self

2d8a5f6

Emulate a single instruction

d77fa82

Bypass capstone 3.0.4 arm bug

e8e2f32

Dealing with capstone

ff98fa4

Temporary disable ASR and remobe BitVec.Bool from test

3b4b60f

WIP WIP debug prints WIP WIP

4c75c8b

Unicorn fallback working (using unicorn master)

942271b

HAck to support unicorn 1.0.0

246df7d

WIP

fda580d

Unicorn hack to handle PC updates

7621fc0

[WIP] do not do anything with this commit; for debugging only

cd6fd72

Adding before clean up

c6f2f30

emulation more or less works; need to work out more unicorn bugs

3a018f1

clean up emulate() caller code

202436a

move hooks to methods; cleanup

b3b6281

Concretize memory when emulating

9a01922

yan added 17 commits April 3, 2017 14:44

Rm unused function

dd390f5

Update concretization; add comments

5eca972

Add ONE policy test

42eecd9

Create a base class for all concretization exceptions

4985733

Remove Armv7Cpu._concretize_registers

53db0ab

Check for enabled logging in a more idiomatic way

4d5d49a

[wip] intermediate testing commit

40c2a24

Reimplement hooks and execution with unicorn

784b1e0

Add a DMB (mem barrier) instruction; nop

5c2b0c0

simplify instruction resolution

1c8cb13

improve unicorn error handling

a08b7ca

explicitly delete emu

6f71378

Handle ARM helpers inline

876acf9

map fetched memory

a5fcf03

redundant comment

728cb3b

update tests

198edab

Narrow exception handling

dc4d756

yan force-pushed the dev-fix-unicorn-hax branch from bb3d7b8 to dc4d756 Compare April 3, 2017 18:44

offlinemark reviewed Apr 3, 2017

View reviewed changes

yan added 2 commits April 3, 2017 15:12

Update DMB docs; make __kuser_dmb match real implementation

2f5e790

post-debug cleanup

74d8f66

offlinemark reviewed Apr 3, 2017

View reviewed changes

manticore/utils/emulate.py Outdated

def _step(self, instruction):

'''

A single attempt at execution an instruction.

Copy link

Contributor

offlinemark Apr 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

offlinemark reviewed Apr 3, 2017

View reviewed changes

feliam reviewed Apr 3, 2017

View reviewed changes

yan added 2 commits April 3, 2017 15:38

Fix typo; add comment; remove extraneous parameter

81e7cfb

typos++

bf0ed9d

offlinemark approved these changes Apr 3, 2017

View reviewed changes

yan merged commit 31b2979 into master Apr 3, 2017

yan deleted the dev-fix-unicorn-hax branch April 3, 2017 20:00

offlinemark mentioned this pull request Apr 4, 2017

Move handleInvalidPC from linux to arm #104

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make unicorn be pull-based #97

Make unicorn be pull-based #97

yan commented Mar 22, 2017

feliam left a comment

feliam Mar 27, 2017

yan Mar 28, 2017

feliam Mar 29, 2017

feliam Mar 31, 2017

feliam Mar 27, 2017

feliam Mar 27, 2017

feliam Mar 27, 2017

feliam Mar 27, 2017

yan Mar 28, 2017

feliam Mar 28, 2017

feliam Mar 28, 2017

yan Mar 28, 2017

offlinemark Mar 28, 2017

feliam Mar 28, 2017

yan Mar 28, 2017

feliam Mar 29, 2017

feliam Mar 28, 2017

yan Mar 28, 2017

feliam Mar 28, 2017

feliam Apr 3, 2017

yan Apr 3, 2017

offlinemark Apr 3, 2017

yan Apr 3, 2017

offlinemark Apr 3, 2017

offlinemark Apr 3, 2017

offlinemark Apr 3, 2017

feliam Apr 3, 2017

Make unicorn be pull-based #97

Make unicorn be pull-based #97

Conversation

yan commented Mar 22, 2017

feliam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment