-
-
Notifications
You must be signed in to change notification settings - Fork 369
librasan: Simplify assembly patches #3192
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
base: main
Are you sure you want to change the base?
Conversation
bbcb655
to
472b5b3
Compare
Nice. Let's land this. |
[0xff, 0xe0].to_vec(), | ||
let addr = destination.to_ne_bytes(); | ||
#[rustfmt::skip] | ||
let insns: &[&[u8]] = &[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random question @WorksButNotTested can't we also do dynasm here like we do in libafl_frida?
LibAFL/libafl_frida/src/cmplog_rt.rs
Line 198 in d3ddc8e
fn generate_instrumentation_blobs(&mut self) { |
Would be more maintable than pasting random bytes, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's pretty nice actually, but dynasm doesn't support ARMv7 and PowerPC. We can migrate the supported architectures if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Nobody loves powerpc! Just went with it as it was the lowest common denominator. Happy if you want to port the others though.
I've added dynasm for the architectures that it supports. |
Actually, scratch that, dynasm requires |
Oh weird. You'd think it would so all its work at compile time! Some crates only support nostd with the default features disabled, does that help at all? |
Dynasm needs some synchronization primitives for the |
BTreeMap is part of core and is a good substitute for HashMap for most usages. That could be an option? |
There is an open PR already: CensoredUsername/dynasm-rs#105. |
It's been idle for quite a while though, sadly. Let's see if they react to the latest comments :) |
There's no alternative crates we can use are there? |
In my opinion, if that nostd PR in dynasm doesn't land soon, we should just revert to using the raw bytes, because I expect very little churn in these assembly trampolines. I don't expect dynasm to start supporting new architectures anytime soon, considering the fair amount of complexity in a full fledged assembler, so we need a fallback anyway. |
Yes let's ignore the move to dynasm for now and maybe keep an eye on their no_std support. |
We could hack together something that emits the right bytes at compile time (build.rs or our own proc macro) and uses a different assembler (or even dynasm) if we wanted |
We could, 'easiest' way would probably to use the correct I still feel that this would be enormous overkill for quite literally 8 to 16 bytes of assembly per target architecture. |
I think the C compiler should detect a .S file as assembly and compile accordingly? As you say though, could be overkill. |
True, but that still leaves |
Description
@domenukk as per your comment in #3176, I've simplified the assembly patches a bit to remove the
Vec
usage. Therustfmt::skip
attributes are there to keep lines matching with the assembly code in the comments.Checklist
./scripts/precommit.sh
and addressed all comments