Fix android shmem #3229
Fix android shmem #3229
Conversation
|
LibAFL/libafl_bolts/src/shmem.rs Line 1205 in 23185b6 Did you see that we have an AshMemShMem we could probably use? |
|
Ah nevermind just saw your description :D |
|
yeah. I'm with @domenukk. Let's fix the providers and use them. The duplication is painful. |
Why do you even need the size of the map for forkserver? It's passed around in env variables IIRC |
|
@domenukk Yes the size of map is indeed useless in forkserver. However, the API of libafl_bolt's shmem need a proper size. And sadly, not all maps' size are passed in env. For example, the size of shared input is nowhere to find (if I understand correctly). |
|
I mean we can just add an extra env if we need to, here (and in libafl): |
| @@ -86,6 +91,30 @@ fn read_u32_from_forkserver() -> Result<u32, Error> { | |||
| Ok(u32::from_ne_bytes(buf)) | |||
| } | |||
|
|
|||
| #[cfg(target_os = "android")] | |||
There was a problem hiding this comment.
these guys has nothing to do with forkserver. so i think it should go into libafl_bolts
There was a problem hiding this comment.
Yes, and this is exactly what the above discussions are about: Shall we reuse the logic in libafl_bolt's shmem, and how?🤔 Since we cannot get size properly, it is not a trivial modification.
There was a problem hiding this comment.
Let's reuse the logic, add an env to the forkserver in libafl and aflpp, if the env is not set, default to the default of MAX_FILE + sizeof(u32) (see link above)
There was a problem hiding this comment.
Sure, make sense. However, I am a bit busy til this weekend. Maybe handle this next week :)
There was a problem hiding this comment.
let's just make that page-aligned MAX_FILE instead of adding 4 bytes
There was a problem hiding this comment.
Nah that's a bad idea for the future
There was a problem hiding this comment.
Inputs might be larger, and everything will have 1 meg hard coded
I mean that's how it is today but it's also bad
There was a problem hiding this comment.
for that part i don't have strong opinion. what i think is bad is that you are trying to mmap for a non-page aligned size.
There was a problem hiding this comment.
Don't think it matters, it's just an extra entry in the page table
The kernel aligns it internally anyway
|
Opened AFLplusplus/AFLplusplus#2430 and #3235 That being said, as @tokatoka said for now we can assume a fixed length / fallback to a fixed length |
|
Merged the env into aflpp and LibAFL, think we can get by without knowing the map size now :) |
|
Nice. I will update this PR next week. |
|
Superseded by #3249 |
Description
Fix #3225.
Ideally, we should reuse the shmem module in libafl_bolts. However, we could not get shared memory size properly (as far as I know) for all target architecture.
Checklist
./scripts/precommit.shand addressed all comments