ensure Nyx command handler support is more CPU agnostic (#3153)

[noperand]

Description

Straightforward fix ensuring the Nyx command handler uses the local arch_regs_map via get_exit_arch_regs(). This is to determine the correct Ret and Cmd CPU registers for the guest architecture, respectively.

No build errors after overriding the guest CPU architecture in Cargo.toml, as described (i386). Same build result when reverting back to arm. Still, must test to be sure.

Checklist

• I have run ./scripts/precommit.sh and addressed all comments
• Test Nyx support for i386 QEMU guest

[rmalmain]

thank you for the PR.

since nyx is specific to x86 targets, i used x86 registers to make sure it is the same as in the nyx api.

the libafl qemu api is meant to be architecture agnostic (and is using arch_regs_map).

is there any specific reason you need the nyx api for the arm architecture?

[noperand]

I've never used Nyx. Seeing #3153 tagged as a bug, I followed the build failure reproduction steps and saw what appeared to be a quick fix based on the specified feature.

I need more clarity on your question about the Nyx API for the arm architecture. The existing example specifies arm but this is designed to be overridden. I could be mistaken, but it seems likely that the reporter is repurposing the QEMU bare metal example for an i386 guest/target. The core Nyx command handler statically referencing RAX will only work for x86_64, not x86. While the registers are synonymous between the two in their lower-level encoding/representation, the compiler is none the wiser with the symbolic enumeration mismatch. The Nyx command handler still needs to reference the right guest register when fetching its contents from QEMU.

The default in the example is currently arm and if what you indicate about Nyx is correct, this sounds like some combination of host/target confusion, feature flag scope/meaning, perhaps an assumption on RAX/EAX being synonymous in the Nyx plumbing, and/or differences at each layer. Regardless, there are now no build errors when building lib_qemu for i386 or arm.

Sounds like there's a better way that may avoid unnecessarily broadening CPU architecture support via feature where it's not meaningful. I'm tracking you there. I do believe it's good to further decouple CPU architecture/register specifics from the Nyx command handler, regardless of limited range of support within Nyx.

I will test this with a proper i386 target and see if anything breaks. If it does, I will provide more updates.

[tokatoka]

can we merge this?
@rmalmain

[domenukk]

LibAFL/libafl_qemu/src/arch/x86_64.rs

Line 45 in f16ddf2

ExitArgs::Cmd => Regs::Rax,

Cmd is not Rbx (should it be?)

[noperand]

Good call out!

Short answer for Nyx; yes. There are other changes needed though; e.g. depend upon Nyx support actually being built (disabled by default in CI/Docker builds, etc already). I no longer think this is the best abstraction to rely upon.

I confirmed that this should be more agnostic; hypercall macros defined in the Nyx packer differentiate between x86 and x86_64 (and of course, emitted code for guests). If I understand correctly, EAX/RAX will vary based on usage of kAFL vs others with and without Intel-PT but changes when trying to avoid collisions with VMware, etc. The Nyx packer code handles more and the code I patched would really be the most common config. The "subcommand" relevant to Nyx appears to be in EBX/RBX and is just a byte, based on the GCC asm modifiers.

I've produced a full Nyx test environment by dual-booting an Intel Core i7 10th Gen. This gets me everything I need; Intel-PT, etc. Everything builds. IIRC I encountered some test failures in other places but had to context switch.

This week, I will prioritize revisiting this PR and can expand further.

[tokatoka]

I consider this PR to be fix to #3153
for that purpose this PR is ready to merge?

[rmalmain]

this was written with only the x64 intel pt usecase indeed, so i'd expect to see issues in another scenario.
the nyx packer always uses [er]ax to detect a nyx hypercall and [er]bx / [er]cx for the arguments.
i don't mind supporting more things that the x64 intel pt mode, but i think it's important we keep the same behavior as the original nyx API, otherwise it makes the main use of this command handler useless.
the original intention was to be able to support nyx targets without having to touch them.
if you have time to revisit the pr it would be appreciated

[tokatoka]

then this file should be disabled for i386 right?

[domenukk]

Maybe use the fix for i386 for people that want/need it, and use the current register assignments for 64 bit?

[tokatoka]

but romain said it's gonna work only with amd64

[tokatoka]

btw now without the restriction of CI runner. i386, arm can be added to CI