Bug in AddressSanitizer instrumentation, broken binary

[alisaesage]

Hi guys,

AddressSanitizer as of Clang 11 on Mac fails to properly instrument linker sets. The resulting asan-instrumented build is unusable due to broken data.

Linker sets is a somewhat unpopular (I only see it used in FreeBSD) but very handy programming technique that uses macros + compiler intrinsics such as __section and __concat to collect object references from multiple source modules, and store them in a dedicated section of the binary, thus exposing them to centralized search and iteration. In binary such a set is represented with an array of pointers that sits in a dedicated binary section and looks like this:

zerodayengineering.com:asan_bug alisa$ objdump --macho -j "__set" asan-bug
asan-bug:
Contents of (__asan_bug_set,__set) section
0000000100003000 10 20 00 00 01 00 00 00 20 20 00 00 01 00 00 00

In the asan-instrumented binary the set is too sparse:

zerodayengineering.com:asan_bug alisa$ objdump --macho -j "__set" asan-bug.asanbuild
asan-bug.asanbuild:
Contents of (__asan_bug_set,__set) section
0000000100003000 60 20 00 00 01 00 00 00 00 00 00 00 00 00 00 00
0000000100003010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0000000100003020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0000000100003030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0000000100003040 a0 20 00 00 01 00 00 00 00 00 00 00 00 00 00 00
0000000100003050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0000000100003060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0000000100003070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

So the asan-instrumented binary fails with a global-buffer-overflow verdict. And if you whitelist it, it fails with a null dereference.

This is used in FreeBSD so I guess it's better fixed.

Attaching the testcase and the asan log.

linker-set-testcase.zip
asan-bug.log

[kcc]

I don't have Mac/FreeBSD dev environments nor skills - asking for help from those who have them.
Or, is there a way to reproduce on linux?
Or, what if you slap __attribute__((no_sanitize("address"))) on some of the globals here?

And if you whitelist it, it fails with a null dereference.
What did you do to whitelist?

[yugr]

Linker sets is a somewhat unpopular (I only see it used in FreeBSD) but very handy programming technique that uses macros + compiler intrinsics such as __section and __concat to collect object references from multiple source modules

I believe kroki/likely uses it as well (and that works on Linux).

[alisaesage]

I don't have Mac/FreeBSD dev environments nor skills - asking for help from those who have them.
Or, is there a way to reproduce on linux?
Or, what if you slap __attribute__((no_sanitize("address"))) on some of the globals here?

I don't think that this is exclusively Mac/FreeBSD specific. It's related to how Asan instruments some Clang general intrinsics. I don't have a PoC for Linux. Actually, I think that the PoC is generic enough – maybe you could try to build it on Linux?

And if you whitelist it, it fails with a null dereference.
What did you do to whitelist?

I did "-fsanitize-blacklist" (an equivalent of -fsanitize-ignorelist for Clang <13), as described in this doc: https://clang.llvm.org/docs/SanitizerSpecialCaseList.html. Specifically I whitelisted the name of the procedure that showed up in the false positive with "global-buffer-overflow".

I am 100% sure that whitelisting is not an option. The problem is that asan instrumentation corrupts an array of pointers in a global section object, thus the program can't iterate over it regardless of any settings.

Based on my understanding of AddressSanitizer internals, the root cause may be that one of the compiler intrinsics involved (please consult the linker set.h header module in my PoC) allocates some global memory for each pointer that it stores in the global section, so ASAN injects some guard space or whatever as part of its global-buffer-overflow instrumentation. As a result, the array/section produced has too much space, that breaks the program when it does "SET_FOREACH" iteration.

[alisaesage]

Linker sets is a somewhat unpopular (I only see it used in FreeBSD) but very handy programming technique that uses macros + compiler intrinsics such as __section and __concat to collect object references from multiple source modules

I believe kroki/likely uses it as well (and that works on Linux).

I think it depends on how exactly the code uses those compiler intrinsics. In my PoC it fails specifically on SET_FOREACH which iterates over the global array, and fails due to extra space injected by Asan instrumentation. If they just do some symbol concatenation and creating new sections, that would pass.

That is to say, I wouldn't exclude that the issue may affect Linux.

[alisaesage]

Or, what if you slap attribute((no_sanitize("address"))) on some of the globals here?

Just tried this on my global structures, no effect.

Maybe it has to be applied to the linker set macros that generate the global pointers?

// linker_set.h
#define __MAKE_SET(set, sym)
__GLOBL(__CONCAT(_start_set,set));
__GLOBL(__CONCAT(_stop_set,set));
static void const * __MAKE_SET_CONST
_set##set##sym##sym section(""#set",__set")
__used = &(sym)

I am not sure where to put it.

[yugr]

I think it depends on how exactly the code uses those compiler intrinsics. In my PoC it fails specifically on SET_FOREACH which iterates over the global array, and fails due to extra space injected by Asan instrumentation. If they just do some symbol concatenation and creating new sections, that would pass.

That is to say, I wouldn't exclude that the issue may affect Linux.

Ah, now I remember. We added -fsanitize-sections=s1,s2,... in GCC specifically to avoid this type of issues (by specifying -fno-sanitize-sections=... user can avoid instrumentation of certain globals like in your case). I'm not sure if there's an equivalent in Clang (it should be very easy to add it though).

[alisaesage]

Ah, now I remember. We added -fsanitize-sections=s1,s2,... in GCC specifically to avoid this type of issues (by specifying -fno-sanitize-sections=... user can avoid instrumentation of certain globals like in your case). I'm not sure if there's an equivalent in Clang (it should be very easy to add it though).

Sounds perfect. There is no such option in clang 11/12 on Mac.

[kcc]

I don't think we have such an option. But its effect should be equivalent to setting attribute((no_sanitize("address"))) for every global variable involved.

[alisaesage]

OK... so how to make my testcase work exactly? I already tried to set attribute((no_sanitize("address"))) on all the globals.

[kcc]

Sorry for the silence.. I'll try to bring someone who knows Mac here.

[kcc]

You may probably try disabling asan for globals completely - in case your primary target is heap/stack and not globals.
-mllvm -asan-globals=0

[yugr]

You may probably try disabling asan for globals completely - in case your primary target is heap/stack and not globals.
-mllvm -asan-globals=0

I'd suggest to adopt GCC's approach of not sanitizing globals in non-standard data sections by default.

[kubamracek]

I can reproduce the problem (on a Mac, but I don't think there's anything Mac-specific in the reproducer, or linker-specific, I think the same should reproduce on Linux too). The suggested workaround of adding the no-asan attribute seems to help and work. I changed this:

#define __MAKE_SET(set, sym) \
	__GLOBL(__CONCAT(__start_set_,set)); \
	__GLOBL(__CONCAT(__stop_set_,set)); \
	static void const * __MAKE_SET_CONST \
	__set_##set##_sym_##sym __section("__"#set",__set") \
	__used = &(sym)

into this:

#define __MAKE_SET(set, sym) \
	__GLOBL(__CONCAT(__start_set_,set)); \
	__GLOBL(__CONCAT(__stop_set_,set)); \
	static void const * __MAKE_SET_CONST \
	__attribute__((no_sanitize("address"))) \
	__set_##set##_sym_##sym __section("__"#set",__set") \
	__used = &(sym)

Is that a good enough solution here? I'm not sure if the -f(no-)sanitize-section flag is on anyone's roadmap.

[yugr]

I think the same should reproduce on Linux too

Definitely.

Is that a good enough solution here?

As a workaround for this particular case - yes. As a general solution, probly not so much.

[kubamracek]

As a general solution, probly not so much.

Why not? Even if we had the -f(no-)sanitize-section flag, it would still require the code/project to be changed to use it. Presumably, it's better to have an explicit annotation in the C source code (it's hard to lose it or forget about it because it transfers if you copy the code around) than to rely on a compilation/linker flag.