falconre / falcon

Binary Analysis Framework in Rust
Apache License 2.0
551 stars 45 forks source link

Lift x86 endbr32/64 and ud0 instructions to Intrinsic #54

Closed emmanuel099 closed 4 years ago

emmanuel099 commented 4 years ago

Handling endbr32/64 instructions fixes some lifting errors when loading binaries compiled with CET enabled.

endeav0r commented 4 years ago

Two things need to happen here.

  1. The correct operation here is Intrinsic. Intrinsic is designed to model instructions that falcon does not handle. This allows analysis to understand the instructions were emitted, but are not modeled. We can then handle them as a special case. For an example of this, see rdwhr handling in finch's handling of MIPS instructions
  2. Tests need to pass.

I can take a look at both of these this week. If you would like to try and tackle these, feel free.

endeav0r commented 4 years ago

I'm taking a look at your fork, and your new introduction of the barrier operation to handle mfence, lfence, and sfence.

One of the things I strove for in Falcon IL is keeping the IL as minimalist as possible. I wanted implementation of analyses to have to consider as few cases as possible. Could these instructions instead be lifted to intrinsics, and handled specifically in your analysis? This would alleviate everyone from having to handle the, "Barrier," case.

emmanuel099 commented 4 years ago

Sorry for the late reply but I was very busy in the past few days.

If you would like to try and tackle these, feel free.

I'm working on it right now. :)

Tests need to pass.

Looks like these enumerators are only available since capstone 0.4.

emmanuel099 commented 4 years ago

Could these instructions instead be lifted to intrinsics, and handled specifically in your analysis?

Yes, definitely! Great idea.

After thinking about it again, lifting all the different fences/barriers to the same "Barrier" instruction isn't a good idea in general, as different analyses may handle them differently.

I'll open another PR for this.

emmanuel099 commented 4 years ago

Thanks for your review! Rebased my changes on top of master.