Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intel MPX support #1152

Closed
wants to merge 3 commits into from
Closed

Intel MPX support #1152

wants to merge 3 commits into from

Conversation

Nitr0-G
Copy link
Contributor

@Nitr0-G Nitr0-G commented Apr 26, 2023

Skipping all MPX instructions has been added in order to eliminate bugs caused on these Issues (#1148 #1135)

Proof that these bugs have been fixed, you can find in the same Issue(#1148 #1135)

Intel MPX is a dead technology that has not been supported by the Linux kernel since 2020(proof: https://www.phoronix.com/news/Intel-MPX-Is-Dead). It was only in the Skylake and Intel Goldmont(atom) architecture, consider all current processors do not support this technology. Zydis & capstone mistakenly disassembles instructions added to Intel MPX(Intel MPX adds 7 new instructions, as well as BND0-3 registers in x64 and x32 mode for more information, see here(https://intel-mpx.github.io/design/ )), a tool like Hiew also does not disassemble instructions of Intel MPX (https://fpic.in/VQ9yfJ1)

Nitr0-G added 2 commits April 26, 2023 21:01
Skipping all MPX instructions has been added in order to eliminate bugs caused on these Issues
avast#1148
avast#1135

Intel MPX is a dead technology that has not been supported by the Linux kernel since 2020(proof: https://www.phoronix.com/news/Intel-MPX-Is-Dead). It was only in the Skylake and Intel Goldmont(atom) architecture, consider all current processors do not support this technology. Zydis & capstone mistakenly disassembles instructions added to Intel MPX(Intel MPX adds 7 new instructions, as well as BND0-3 registers in x64 and x32 mode for more information, see here(https://intel-mpx.github.io/design /)), a tool like Hiew also does not disassemble instructions of Intel MPX (https://fpic.in/VQ9yfJ1)
Skipping all MPX instructions has been added in order to eliminate bugs caused on these Issues (avast#1148
avast#1135)

Intel MPX is a dead technology that has not been supported by the Linux kernel since 2020(proof: https://www.phoronix.com/news/Intel-MPX-Is-Dead). It was only in the Skylake and Intel Goldmont(atom) architecture, consider all current processors do not support this technology. Zydis & capstone mistakenly disassembles instructions added to Intel MPX(Intel MPX adds 7 new instructions, as well as BND0-3 registers in x64 and x32 mode for more information, see here(https://intel-mpx.github.io/design/ )), a tool like Hiew also does not disassemble instructions of Intel MPX (https://fpic.in/VQ9yfJ1)
@PeterMatula
Copy link
Collaborator

PeterMatula commented Apr 27, 2023

Thanks for the investigation of the problem, however:

  • Looks like the tests are failing for your change, see point below...
  • This (i.e. Capstone2LlvmIrTranslator_impl::translateOne()) is not the right place where to add such code like you did. This method is meant to implement instruction translation on high-level. I.e. regardless the concrete instruction(s) (like X86_INS_BND* here) or architecture. By modifying the code here, you not only did not do it on the right place for X86, but you also affected translation of instructions for all the other architectures. Those X86_INS_BND* are Capstone's x86_insn enum items with an integer value. Their value may, and in fact it certainly does, conflict with values for other architectures like arm_insn. If you then do if (insn[0].id == MpxIndexer) you are gonna affect translation of all the other instructions - as instructions for all the architectures are handled by the Capstone2LlvmIrTranslator_impl::translateOne() method.
  • Specific code for specific architectures comes to their modules like src/capstone2llvmir/{arm, arm64, x86, ...}. If you want to modify translation of x86 instruction, this is the general place to modify.
  • Even there, your modification would not be the best way to do it. You don't want to make this kind of hackish fix on a method that processes all the instruction irrespective of their particular type. Here is the code that controls instruction translation - it assigns a translation routine to every instruction (looks like assignments for those X86_INS_BND* are missing, this is probably caused by migrating this code from older Capstone version which did not have them to a newer version which does). Either a specific method is assigned to an instruction, or a method handling multiple instructions, or translateNop() if instruction is to be ignored, or nullptr which should trigger a best-effort automated translation.
  • What probably happened in Error "Missing type for register number". MPX technology is dead. BND registers are trying to be decoded, although they are not supported #1148 is the triggering of best-effort pseudo-instruction translation. And it failed, because it automatically tried to use registers, or whatever, which were not generated (again, probably because they were not in old Capstone version).
  • So, if you want to generate nothing for X86_INS_BND*, simply add entries for these instructions here and map them to &Capstone2LlvmIrTranslatorX86_impl::translateNop e.g. as for X86_INS_UD1:
    {X86_INS_UD1, &Capstone2LlvmIrTranslatorX86_impl::translateNop},
  • While this might be an fine translation for these instructions, it would still be best to also implement register generation, if there are some registers in Capstone, which are not handled by capstone2llvmir. Register translation is handled by the same source code. The most important thing to fill is this map and they should ideally by generated and usable. However, x86 registers are sometimes tricky and a special handling needs to be implemented - it depends, have a look around that initialization source code to see what I mean.

I write all of this in case you want to properly fix your PR and understand the source code better. If you don't necessarily want that, let me know and I will do the fix myself. Thanks to your investigation of the problem it should be a relatively easy task for me 👍

@Nitr0-G
Copy link
Contributor Author

Nitr0-G commented Apr 27, 2023

I write all of this in case you want to properly fix your PR and understand the source code better. If you don't necessarily want that, let me know and I will do the fix myself. Thanks to your investigation of the problem it should be a relatively easy task for me

Thank you very much! I will try to fix it during this and the following days! I'm really interested in understanding the source code, so I'll try to do it myself

I also found a problem with static analysis of virtualized calls such as MessageBox, etc. WinAPI with passing arguments. A little later I will also make an issue about this.

@Nitr0-G Nitr0-G closed this Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants