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

Binaries linked with shared ASAN behave different on various distributions #62431

Open
maxammann opened this issue Apr 28, 2023 · 13 comments
Open
Labels
compiler-rt:asan Address sanitizer

Comments

@maxammann
Copy link

maxammann commented Apr 28, 2023

It is possible to link ASan statically or dynamically. The default in Clang is static, and dynamic is to a certain degree experimental. On GCC (which uses compiler-rt) the default is dynamic and it is a stable feature.

While fuzzing with AFLplusplus I discovered a discrepancy on how it behaves on Ubuntu vs. Debian/Fedora/Nix. I raised an issue in AFL++, but we are unsure what and where the proper fix should be implemented.

A very similar issue exists with __asan_default_options, though no fix was proposed.

Reproduction

The following experiments are performed on Debian and NOT on Ubuntu. See below for more information.

Suppose the following harness, which copies from outside an allocated region to some other buffer (y).

#include <string.h>
#include <stdio.h>
#include <stdlib.h>
int main() {
  char *x = (char*)malloc(5 * sizeof(char));
  memset(x, 0, 5);
  char *y = (char*)malloc(10 * sizeof(char));
  memcpy(y, x, 10);
  free(x);
  free(y);
  return 0;
}

__attribute__((weak)) void *__asan_region_is_poisoned(void *beg, size_t size) {
  (void)beg;
  (void)size;
  return NULL;
}

(__asan_region_is_poisoned is for instance defined in AFL++ and other tools. See issue on how.)

Compiling and executing this with ASan should crash. And it is true in the default configuration.

Command Corruption detected?
clang harness.c -o harness -O0 -fsanitize=address && ./harness
Command Corruption detected?
clang harness.c -o harness -O0 -fsanitize=address -shared-libasan && LD_PRELOAD=$(clang -print-file-name=libclang_rt.asan-x86_64.so) ./harness

The reason for this is that libclang_rt.asan-x86_64.so exports __asan_region_is_poisoned and makes it relocatable on Debian. On Ubuntu the symbol is exported but not relocatable, and therefore the correct implementation from libclang_rt.asan-x86_64.so will be chosen.

With GCC shared ASan is the default. And it works with static and shared ASan.

Command Corruption detected?
gcc harness.c -o harness -O0 -fsanitize=address && ./harness
gcc harness.c -o harness -O0 -fsanitize=address -static-libasan && ./harness

Why is shared ASan + Clang not working on Debian?

On Debian the libclang_rt.asan-x86_64.so exports __asan_region_is_poisoned as dynamic relocatable. On Ubuntu it is not relocatable.
The impact is that on Debian, ASan will use the stub __asan_region_is_poisoned defined by the main binary.
On Ubuntu, ASan will use the __asan_region_is_poisoned from ASan, because it is statically referenced.

This can be checked using the following command:

$ objdump --dynamic-reloc $(clang -print-file-name=libclang_rt.asan-x86_64.so) | grep __asan_region_is_poisoned
# Debian: 00000000001091c0 R_X86_64_JUMP_SLOT  __asan_region_is_poisoned@@Base
# Ubuntu: not found 

Why does this behavior only show on Debian and not Ubuntu?

Like already described, the output of objdump --dynamic-reloc $(clang -print-file-name=libclang_rt.asan-x86_64.so) is different on Ubuntu and Debian.

The reason is that dpkg-buildflags --get LDFLAGS includes -Wl,-Bsymbolic-functions, which avoids that exported symbols are made relocatable during linking. That way on Ubuntu libclang_rt.asan-x86_64.so contains no relocatable functions.

Proposed fix

Enable -Bsymbolic-functions for compiler-rt directly in LLVM, so the behavior is identical across distributions. This has already been done for clang-shlib here.

Even though Ubuntu enabled this flag globally for performance reasons, it is actually the better behavior for shared ASAN, because it will avoid that binaries can "overwrite" functions which are internal to ASan.

Another options would be to drastically limit which functions ASan exports (this will also remove it from the relocation table). Though, this might not be the intended thing to do because certain internal functions are already called by tools.

References:

@maxammann
Copy link
Author

@hahnjo pinging you here because you issues the original report on the mailing list :)

https://www.mail-archive.com/ubuntu-bugs@lists.ubuntu.com/msg6005262.html

@MaskRay
Copy link
Member

MaskRay commented Apr 28, 2023

Most sanitizer functions are not intended to be interposed. Do you know why AFLplusplus does https://github.com/AFLplusplus/AFLplusplus/blob/7ca1b85c5e8229fa49620d0fb542c86965ef5abb/instrumentation/afl-compiler-rt.o.c#L1999-L2003 ? This is a weak definition and will lose to libclang_rt.asan.a.

For -shared-libsan, the interposition only works when libclang_rt.asan.so is not linked with -Wl,-Bsymbolic-functions (or similar options).

https://www.mail-archive.com/ubuntu-bugs@lists.ubuntu.com/msg6005262.html

__asan_default_options not working with -shared-libasan

You get a non-interposable __asan_default_options in libclang_rt.asan.so if you use -Wl,-Bsymbolic-functions.

If you want to interpose it (weak definition in libclang_rt.asan.so) in the executable while making other __asan_* functions non-interposable, use -Wl,-Bsymbolic-non-weak-functions (lld). GNU ld doesn't implement the option. (I have a patch that they don't take https://sourceware.org/bugzilla/show_bug.cgi?id=27871).

@maxammann
Copy link
Author

Sadly I don't understand yet why AFL++ requires that.

Interesting! I think one thing which would be great to define is whether certain functions should be interposable or not and somehow enforce that downstream.

@vanhauser-thc
Copy link

We have an object file that is compiled into targets.
If a target has asan then we need to check if a region is poisoned.
if a target is not compiled with asan our weak definition simply does nothing.
without defining the function our code would not link (symbol cannot be resolved).

I am open to other ideas how to solve that in our code.
However that Debian and Ubuntu compile llvm differently so that different things happen is the source of the problem IMHO.

@maxammann
Copy link
Author

Yes, I think the first thing this should be adressed is to enforce from upstream that the "
"interposition of functions" is the same across distros. E.g. by setting something like -Wl,-Bno-symbolic-functions from LLVM.

Then shared ASAN will be broken fro the AFL++ usecase, but I guess that is fine.

The -Wl,-Bsymbolic-non-weak-functions would be interseting if I understood correctly, though. It will be along way though until GCC supports it and we reach equality across distros.

@disconnect3d
Copy link

I am open to other ideas how to solve that in our code.

Since ASAN should be enabled by providing the -fsanitize=address as both linker and compiler flags, maybe we can add or remove this function based on __SANITIZE_ADDRESS__ flag being defined? Or would that not work because e.g. the object file compiled must be able to work with both sanitized or not sanitized builds? (Though shouldn't its code also get instrumented by ASAN or is that unwanted or disabled here?)

@vanhauser-thc
Copy link

vanhauser-thc commented May 1, 2023

sadly that is not possible because the runtime is built once and never again, and afterwards has to be linked with any target, asan or not.

@hahnjo
Copy link
Member

hahnjo commented May 3, 2023

But is it a problem if __asan_region_is_poisoned is defined strongly and just not active without sanitizer? As mentioned above, it really should be a strong definition to win against any weak fallback.

That said, it doesn't solve the problem I reported here: https://bugs.launchpad.net/ubuntu/+source/llvm-defaults/+bug/1964487

@maxammann
Copy link
Author

But is it a problem if __asan_region_is_poisoned is defined strongly and just not active without sanitizer?

This is not a problem, it is just like an unused symbol in the binary.

I agree that your problem is not solved and indeed we should make sure that compiler-rt is not compiled with any -B option.

This will break shared ASAN for AFL++ ok Ubuntu (just like on any other distribution.

Later we should figure out how to support the linker-based ASAN detection which AFL++ uses.

@vanhauser-thc
Copy link

for those interested, can you please test https://github.com/AFLplusplus/AFLplusplus/tree/gccasan ?

@yugr
Copy link
Member

yugr commented Feb 1, 2024

We have an object file that is compiled into targets. If a target has asan then we need to check if a region is poisoned. if a target is not compiled with asan our weak definition simply does nothing. without defining the function our code would not link (symbol cannot be resolved).

Have you considered using dlsym instead to check that symbol is present? That way you wouldn't need the default implementation.

@yugr
Copy link
Member

yugr commented Feb 1, 2024

sadly that is not possible because the runtime is built once and never again, and afterwards has to be linked with any target, asan or not.

BTW is splitting AFL runtime an option (so that Asan part is linked only under -fsanitize=address)?

@maxammann
Copy link
Author

I reported this to Ubuntu because I believe that is the best place to fix it: https://bugs.launchpad.net/ubuntu/+source/clang/+bug/2052954

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt:asan Address sanitizer
Projects
None yet
Development

No branches or pull requests

7 participants