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

libafl_qemu: Add RISCV support #2367

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

saibotk
Copy link
Contributor

@saibotk saibotk commented Jul 8, 2024

Adds the following targets (as features):

  • riscv32
  • riscv64

Added RISCVCPU and CPURISCVState to the bindings allow list.

Added riscv.rs to the arch module, with all necessary functions and registers implemented and mapped.
The registers are the same as the ones found in qemus gdbstub xml found after a build.

Additionally, we added all syscall numbers for riscv 64 bit (already supported by the syscall_numbers crate) and also added the missing ones for riscv 32 bit. We compared both lists and their differences / equalities with a simple python script and generated a list of the missing ones, to be complete.
We might PR those to the syscall_numbers crate later on.

PR in the qemu-libafl-bridge repository:
AFLplusplus/qemu-libafl-bridge#77

Note: After that one got merged, the qemu commit sha still has to be adjusted.

Adds the following targets (as features):
- riscv32
- riscv64

Added `RISCVCPU` and `CPURISCVState` to the bindings allow list.

Added riscv.rs to the arch module, with all necessary functions and
registers implemented and mapped.
The registers are the same as the ones found in qemus gdbstub xml found
after a build.

Additionally we added all syscall numbers for riscv 64 bit (already
supported by the `syscall_numbers` crate) and also added the missing
ones for riscv 32 bit. We compared both lists and their differences /
equalities with a simple python script and generated a list of the
missing ones, to be complete.
We might PR those to the `syscall_numbers` crate later on.
@rmalmain
Copy link
Collaborator

rmalmain commented Jul 8, 2024

looks good to me, thanks for the contribution! I'll have a closer look during the week, i have little time until wednesday.

@rmalmain
Copy link
Collaborator

I'll wait for #2267 to be merged before merging this, it's quite heavy refactoring so i think it'll make things simpler.
since #2380 is completing this, I'll have a look after both PRs are merged.

@saibotk
Copy link
Contributor Author

saibotk commented Jul 19, 2024

@rmalmain I noticed that capstone also provides an extra mode for the Compressed RISCV extension and I would like to support this via a feature flag.

This would be useful because some other helpers do also instantiate capstone via the provided method for each architecture.

I'd suggest adding a compressed_instructions feature flag, to then enable the mode in riscv.rs and maybe also for MIPS? Because they have an equivalent setting (https://docs.rs/capstone/latest/capstone/arch/mips/enum.ArchExtraMode.html).

Do you think that would be useful or something you would consider to be added?
If so I would update this PR to reflect that or create a follow-up if that's preferred :)

let me know

@rmalmain
Copy link
Collaborator

hi @saibotk, i think we can add compressed instruction support on top of your PR.
however, i'm not sure we need to add a specific feature for this. since it's an extension of the assembly compatible both with 32 and 64 bits from what i understand (i never really looked at the risc v assembly, so take what i say with a grain of salt).

i would prefer to keep architectural-related features for things that cannot easily be set at runtime (like choosing the qemu target architecture) or would help to gain performance significantly. otherwise, i believe it's much better to have a builder and set things in rust directly.

for example, i don't think we should keep the be or le as features. i believe it would make more sense to make this a run-time thing since some architecture can even switch endianness at runtime.

is there a reason not to enable the C extension by default btw?

@saibotk
Copy link
Contributor Author

saibotk commented Jul 19, 2024

Oh okay yes that sounds like a plan i also felt like that would be better off in a runtime config though we need to make it possible to extend the capstone initialization in the qemu modules. But lets worry about that in another PR.

Oh what C Extension are you talking about?

@rmalmain
Copy link
Collaborator

The one in the risc-v specification, chapter 26.

@saibotk
Copy link
Contributor Author

saibotk commented Jul 20, 2024

Oh you mean the compressed mode for capstone, i have not looked into it deeply and it could be fine to do so, my only concern would be that it could then detect instructions in rubbish data which it would not if the mode was disabled.

@domenukk
Copy link
Member

@rmalmain status?

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.

3 participants