tsan: tolerate munmap with invalid arguments
We call UnmapShadow before the actual munmap, at that point we don't yet
know if the provided address/size are sane. We can't call UnmapShadow
after the actual munmap becuase at that point the memory range can
already be reused for something else, so we can't rely on the munmap
return value to understand is the values are sane.
While calling munmap with insane values (non-canonical address, negative
size, etc) is an error, the kernel won't crash. We must also try to not
crash as the failure mode is very confusing (paging fault inside of the
runtime on some derived shadow address).
Such invalid arguments are observed on Chromium tests:
https://bugs.chromium.org/p/chromium/issues/detail?id=1275581
Reviewed By: melver
Differential Revision: https://reviews.llvm.org/D114944
GitOrigin-RevId: 1b576585eba4d08d5c97c39f5f43d0bf6eed4e4e
diff --git a/lib/tsan/rtl/tsan_rtl.cpp b/lib/tsan/rtl/tsan_rtl.cpp
index 6ff52e3..c14af97 100644
--- a/lib/tsan/rtl/tsan_rtl.cpp
+++ b/lib/tsan/rtl/tsan_rtl.cpp
@@ -274,8 +274,39 @@
}
#if !SANITIZER_GO
+// We call UnmapShadow before the actual munmap, at that point we don't yet
+// know if the provided address/size are sane. We can't call UnmapShadow
+// after the actual munmap becuase at that point the memory range can
+// already be reused for something else, so we can't rely on the munmap
+// return value to understand is the values are sane.
+// While calling munmap with insane values (non-canonical address, negative
+// size, etc) is an error, the kernel won't crash. We must also try to not
+// crash as the failure mode is very confusing (paging fault inside of the
+// runtime on some derived shadow address).
+static bool IsValidMmapRange(uptr addr, uptr size) {
+ if (size == 0)
+ return true;
+ if (static_cast<sptr>(size) < 0)
+ return false;
+ if (!IsAppMem(addr) || !IsAppMem(addr + size - 1))
+ return false;
+ // Check that if the start of the region belongs to one of app ranges,
+ // end of the region belongs to the same region.
+ const uptr ranges[][2] = {
+ {LoAppMemBeg(), LoAppMemEnd()},
+ {MidAppMemBeg(), MidAppMemEnd()},
+ {HiAppMemBeg(), HiAppMemEnd()},
+ };
+ for (auto range : ranges) {
+ if (addr >= range[0] && addr < range[1])
+ return addr + size <= range[1];
+ }
+ return false;
+}
+
void UnmapShadow(ThreadState *thr, uptr addr, uptr size) {
- if (size == 0) return;
+ if (size == 0 || !IsValidMmapRange(addr, size))
+ return;
DontNeedShadowFor(addr, size);
ScopedGlobalProcessor sgp;
ctx->metamap.ResetRange(thr->proc(), addr, size);
diff --git a/test/tsan/munmap_invalid.cpp b/test/tsan/munmap_invalid.cpp
new file mode 100644
index 0000000..ce8588d
--- /dev/null
+++ b/test/tsan/munmap_invalid.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+
+#include "test.h"
+#include <sys/mman.h>
+
+int main() {
+ // These bogus munmap's must not crash tsan runtime.
+ munmap(0, 1);
+ munmap(0, -1);
+ munmap((void *)main, -1);
+ void *p =
+ mmap(0, 4096, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0);
+ munmap(p, (1ull << 60));
+ munmap(p, -10000);
+ munmap(p, 0);
+ fprintf(stderr, "DONE\n");
+ return 0;
+}
+
+// CHECK: DONE
