librasan: Use musl implementations of some libc functions

[wfdewith]

Description

As discussed in #3173, this PR replaces all libc functions required to compile librasan with their implementations from musl. This also fixes the unaligned memory access in memset that #3173 was intended to fix.

Musl does offer handwritten optimized versions in assembly for some of these functions for some architectures, but these implementations might overread buffers, which would trip up the address sanitizer on its own code. I have no interest at the moment to manually verify and tweak the assembly code (and test those adjustments thoroughly!), so I'm leaving them out for now.

Checklist

• I have run ./scripts/precommit.sh and addressed all comments

[wfdewith]

I've also added a .clang-format-ignore to ignore the files that have been copied from musl to preserve their formatting, but you might prefer to do it through the regex below.

LibAFL/utils/libafl_repo_tools/src/main.rs

Lines 296 to 307 in 9771891

	let c_excluded_directories = RegexSet::new([
	r".*target.*",
	r".*libpng-1\.6.*",
	r".*stb_image\.h$",
	r".*dlmalloc\.c$",
	r".*QEMU-Nyx.*",
	r".*AFLplusplus.*",
	r".*Little-CMS.*",
	r".*cms_transform_fuzzer.cc.*",
	r".*sqlite3.*",
	r".*libfuzzer_libmozjpeg.*",
	])

[WorksButNotTested]

Nice. Looks good.

[domenukk]

Thanks!

Why put the libc stuff in a cc folder? cc is usually the name for our compiler wrapper things (see libafl_cc).
Some other name like musl, libc, stdlib, ... would be preferrable

[WorksButNotTested]

To be fair to @wfdewith the folder was already there in the original implementation, he just added some files in there. I don't have any problem with it being renamed though.

[wfdewith]

I prefer libc as the name as well, and it better documents the purpose of the included C code.

[wfdewith]

I double checked whether the optimized C implementations don't suffer from buffer overreads similarly to the assembly implementions, as mentioned above.

• ✔️ memcmp checks both buffers per character, so no issue.
• ✔️ memcpy reads the source buffer per word, but only if the entire word is part of the buffer. The trailing bytes are read per byte. As long as the implementation is correct, can never write past the end of the buffer.
• ✔️ memmove reads the source buffer per word, but only if the entire word is part of the buffer. The trailing bytes are read per byte. As long as the implementation is correct, can never write past the end of the buffer.
• ✔️ memset does not read any memory, and as long as the implementation is correct, can never write past the end of the buffer.
• ❌ strlen walks the source buffer per word. Since there is no predetermined length, it leads to an ASan violation if the null terminator is not the last byte of a word.

Luckily, the optimized part of strlen is easy to remove.

[WorksButNotTested]

The asan library itself is excluded when injecting TCG to check for overruns. Therefore such benign over-reads from load widening shouldn't be any issue here.

[wfdewith]

The asan library itself is excluded when injecting TCG to check for overruns. Therefore such benign over-reads from load widening shouldn't be any issue here.

I know this is the case for AsanGuestModule but I couldn't find such an exclusion for AsanModule.

[WorksButNotTested]

I've not tried librasan with system mode, I only designed it for use with user mode qemu. I guess the modular nature might mean that parts can be re-used? Maybe just add a comment warning of the potential problem of load widening and keep the current optimised implementation? Then if it is integrated with system mode later, it should be easy to detect and fix?

[wfdewith]

I've not tried librasan with system mode, I only designed it for use with user mode qemu. I guess the modular nature might mean that parts can be re-used? Maybe just add a comment warning of the potential problem of load widening and keep the current optimised implementation? Then if it is integrated with system mode later, it should be easy to detect and fix?

I don't understand, I thought the difference between AsanModule and AsanGuestModule is just whether the shadow memory is managed in QEMU through syscalls or in the guest process itself. Both are usermode QEMU as far as I can see.

Also keep in mind that this strlen function will not be called by the actual program under test, it's just needed for the Rust standard library CStr, which is only used with a couple of static strings in librasan.

[WorksButNotTested]

Ah gotcha. Sorry got confused by the naming. I think AsanModule (qemu-user but with the shadow maps in the host) should also exclude the DSO from instrumentation. qemu_launcher also supports configuration of excluded ranges via command line options. That should also work for both guest and host modes. That's not to say that is what it does though.

[wfdewith]

Yeah, it's just not excluded in AsanModule here:

LibAFL/libafl_qemu/src/modules/usermode/asan.rs

Lines 1162 to 1181 in 62d9485

	pub fn gen_readwrite_asan<ET, I, S>(
	_qemu: Qemu,
	emulator_modules: &mut EmulatorModules<ET, I, S>,
	_state: Option<&mut S>,
	pc: GuestAddr,
	_addr: *mut TCGTemp,
	_info: MemAccessInfo,
	) -> Option<u64>
	where
	ET: EmulatorModuleTuple<I, S>,
	I: Unpin,
	S: Unpin,
	{
	let h = emulator_modules.get_mut::<AsanModule>().unwrap();
	if h.must_instrument(pc) {
	Some(pc.into())
	} else {
	None
	}
	}

But it is in AsanGuestModule:

LibAFL/libafl_qemu/src/modules/usermode/asan_guest.rs

Lines 118 to 126 in 62d9485

	/* Don't sanitize the sanitizer! */
	if let Some(asan_mappings) = &h.asan_mappings {
	if asan_mappings
	.iter()
	.any(|m| m.start() <= pc && pc < m.end())
	{
	return None;
	}
	}

I really can't think of a reason why it shouldn't be handled exactly the same in both modules. Should I open a PR for that?

I guess that if load widening isn't a problem, I could also include the handwritten optimized assembly implementations from musl, because why would we leave performance on the table.

[WorksButNotTested]

Yeah. Please PR that. No reason not to have the optimised versions i guess. I'd be inclined to put them behind a feature (that's enabled by default) just in case they do cause some problems so integrations can opt out.

[WorksButNotTested]

By the way, you should be able to profile your target running in the "runner" (included with librasan) to find any more potential areas for improvement. I used samply along with the Firefox-profiler.

[WorksButNotTested]

Actually, thinking about it, I don't think this is the first time I've had to implement memcpy etc. It might be useful to put it in a standalone re-usable crate and publish it?

[wfdewith]

I was actually thinking about whether it is possible to compile the librasan crates with Rust's *-musl targets. It would give us the entire libc statically linked for free, including the hand-optimized assembly implementations of some functions. We could even replace some patch_ functions (such as patch_strlen) with a call to the libc crate to take advantage of that in the program under test as well. It would also reduce maintenance overhead if the Rust standard library ever changes which libc functions it uses.

We'd need to do some linker tricks to avoid linking dlsym and dlerror against musl's implementations of these functions, but that should be very doable. Other than that, I don't expect any significant issues, but maybe I'm overlooking something.

[WorksButNotTested]

The only issue I can see is availability of rust target support for musl. What can be done for target architectures where there is no musl rust target?

[WorksButNotTested]

Can musl be added as a dependency in the Cargo.toml rather than as part of the platform specification?

[WorksButNotTested]

Also it could cause problems when using it on bare metal systems.

[wfdewith]

The only issue I can see is availability of rust target support for musl. What can be done for target architectures where there is no musl rust target?

Good point, actually, I assumed this was more supported than it actually is.

Looking at Platform Support, the following targets exist in Tier 2:

• aarch64-unknown-linux-musl
• loongarch64-unknown-linux-musl
• powerpc64le-unknown-linux-musl
• riscv64gc-unknown-linux-musl
• x86_64-unknown-linux-musl
• armv7-unknown-linux-musleabi
• i686-unknown-linux-musl

So this would already be a problem for PowerPC as it stands, because PowerPC 64 LE is not the same as PowerPC. There is Tier 3 support for PowerPC (and PowerPC 64) with musl and for a couple other architectures with musl as well, but those require building your own rustc and Rust standard library. There is also no guarantee that these targets build and work correctly.

Can musl be added as a dependency in the Cargo.toml rather than as part of the platform specification?

Not really, there is no existing crate for this because it is normally bundled with the standard library anyway. Setting up a build environment for creating such a crate would be pretty complex and that time would probably be better spent just compiling the Tier 3 targets and getting them working.

Also it could cause problems when using it on bare metal systems.

I wouldn't expect trouble, since we're compiling the whole thing with no_std already anyway.

---

Anyway, it seems that this idea is dead on arrival because of PowerPC, so let's disregard it for now and just use the few select functions from musl.

Actually, thinking about it, I don't think this is the first time I've had to implement memcpy etc. It might be useful to put it in a standalone re-usable crate and publish it?

Maybe, but I prefer leaving that to you because you have a better idea of what you need.

---

So, my current plan is as follows:

• Create and land a PR that addresses the lack of filtering of the librasan address space in AsanModule. (Exclude ASAN DSO address ranges in QEMU AsanModule #3180)
• Restore the strlen implementation in this PR.
• Include the handwritten assembly implementations in this PR as well, since load widening shouldn't be an issue anymore.
• Create a new PR that uses these C or assembly implementations in their respective patch_ functions, if possible. This depends on what kind of error checking is performed in the patch_ functions. It can at least be done for strlen and memcmp.
• Optionally create a microbenchmark suite for the hooked functions to actually have some data to talk about instead of guessing what performance characteristics will be.

[WorksButNotTested]

Sounds good. Only thing I'd add is putting the assembly implementations behind a default enabled feature in case there's issues with them on exotic targets.

[WorksButNotTested]

How does this look? https://crates.io/crates/nostd-musl

[wfdewith]

I realized that the Rust compiler already can almost do what we're trying to achieve when it compiles for freestanding targets. For that purpose, rustc already has implementations for the required memory related libc functions. See https://github.com/rust-lang/compiler-builtins/blob/master/compiler-builtins/src/mem/mod.rs. However, the build system of the compiler_builtins crate is complex and entirely focused on bundling it with rustc, so using it directly is probably not possible.

compiler_builtins appears to only have optimized versions of the mem-related functions for x86_64, so it is not that interesting to us for copying faster implementations of these functions either.

Long story short, the current idea, and as such your crate, seems to be the best fit for this project. Should this PR be ported to that crate?

[WorksButNotTested]

I think so, but worth seeing what the rest of the maintainers think.

[domenukk]

Sounds good to me

[WorksButNotTested]

Great thanks. Happy to give anyone else privs for the repo or crates.io BTW. Or even move the repo into AFLplusplus org if you'd rather. Just didn't want to necessarily tie the release schedule of the crate to that of libafl.

[domenukk]

Whatever you like best :)

[WorksButNotTested]

Ok. I'll leave it as is for now then if nobody feels strongly either way.

[WorksButNotTested]

@wfdewith I've just made this. #3188. Does it look ok to you?

[WorksButNotTested]

Ok this should be good to supersede this commit now. #3188. Happy if you want to spin up a new PR to integrate calls into the new nostd-musl crate (or indeed nostd-printf) into the hook as required.