[hwasan] Handle possible failure of dladdr

[mmalcomson]

Summary

In `GetGlobalSizeFromDescriptor` we use `dladdr` to get info on the the
current address.  `dladdr` returns 0 if it failed.
During testing on Linux this returned 0 to indicate failure, and
populated the `info` structure with a NULL pointer which was
dereferenced later.

This patch checks for `dladdr` returning 0, and in that case returns 0
from `GetGlobalSizeFromDescriptor` to indicate failure of identifying
the address.

N.b. the testcases where this failed were most tests using stack-tagging
with glibc and using the interception ABI.
I have not been able to trigger the problem any other way, which is why
there is no testcase here.

[eugenis]

Any idea how this is possible?
We only call this if GetModuleNameAndOffsetForPC succeeds, which should only happen when an address is within a shared object.

[hctim]

This looks reasonable to me, but like Evgenii I don't understand how this happened. Was there a separate thread that did a racy dlclose()?

[mmalcomson]

The problem is when hwasan detects a fault accessing a value on the stack in Linux.
What I believe happens is:

On Linux GetModuleNameAndOffsetForPC succeeds for the stack.

It uses FindModuleForAddress, which uses fallback_modules_, fallback_modules_ uses fallbackInit, which initialises the list of modules from /proc/self/maps.

On Linux /proc/self/maps has a mapping for the stack -- which means the fallback_modules_ has a module named "[stack]".

Hence GetModuleNameAndOffsetForPC returns the name "[stack]" and the offset into it (and dladdr doesn't work on a stack address).

I guess we could compare against specially named modules like "[stack]" and "[heap]" by checking for the first character of the module name, but I feel this check against dladdr failing would be more robust.

[eugenis]

LGTM
Thank you.

[hctim]

Thanks for the clarification!

Just returning 0 from GetGlobalSizeFromDescriptor will result in HWASan also printing XXX is located to the right/left of a global variable in (foo.so+0x1234) for stack tag mismatches.

I guess we could compare against specially named modules like "[stack]" and "[heap]" by checking for the first character of the module name, but I feel this check against dladdr failing would be more robust.

I'm not sure this works on Android - as any pages in the bss segment that don't come from the pages occupied by the file mapping end up being mapped as [anon:bss]. I was going to say that maybe we can get away with ignoring non-fd based mappings, but this is incompatible with things that use anonymous maps for file-based content (like MTE globals + UBSan might have problems if UBSan needs the maps somewhere).

Thanks for pointing out this bug - I've just realised the same problem exists on Android as well. I'm not sure how we didn't catch the regression introduced here, all stack-based reports are cut off really short.

For now I'll approve this patch to unblock, and I'll circle back and rework slightly here to remove the unnecessary 0x007feca724bc is located to the left of a global variable in ([stack]+0x1f4c0).

compiler-rt/lib/hwasan/hwasan_report.cpp

 
 // HWASan globals store the size of the global in the descriptor. In cases where
 // we don't have a binary with symbols, we can't grab the size of the global
 // from the debug info - but we might be able to retrieve it from the
 // descriptor. Returns zero if the lookup failed.
 static uptr GetGlobalSizeFromDescriptor(uptr ptr) {
   // Find the ELF object that this global resides in.
   Dl_info info;
-  dladdr(reinterpret_cast<void *>(ptr), &info);
+  if (dladdr(reinterpret_cast<void *>(ptr), &info) == 0)
+    return 0;
   auto *ehdr = reinterpret_cast<const ElfW(Ehdr) *>(info.dli_fbase);
   auto *phdr_begin = reinterpret_cast<const ElfW(Phdr) *>(
       reinterpret_cast<const u8 *>(ehdr) + ehdr->e_phoff);
 
   // Get the load bias. This is normally the same as the dli_fbase address on
   // position-independent code, but can be different on non-PIE executables,
   // binaries using LLD's partitioning feature, or binaries compiled with a
   // linker script.