1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
|
From b1bd52cd0d8627df1187448b8247a9c7a4675019 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston@google.com>
Date: Wed, 12 Apr 2023 20:53:49 +0000
Subject: [PATCH] Fix tls_get_addr handling for glibc >=2.25
This changes the sanitizers' tls_get_addr handling from
a heuristic check of __signal_safe_memalign allocations
(which has only been used in a since deprecated version
of Google's runtime), to using the sanitizers' interface
function to check if it is a malloc allocation (used
since glibc >= 2.25).
This is one of the approaches proposed by Keno in
https://github.com/google/sanitizers/issues/1409#issuecomment-1214244142
This moves the weak annotation of __sanitizer_get_allocated_size/begin from the header to sanitizer_tls_get_addr.cpp, as suggested by Vitaly in D148060.
Reviewed By: vitalybuka
Differential Revision: https://reviews.llvm.org/D147459
---
.../sanitizer_allocator_interface.h | 4 +--
.../sanitizer_tls_get_addr.cpp | 29 ++++++++++---------
.../sanitizer_common/sanitizer_tls_get_addr.h | 26 +++++++++++------
compiler-rt/test/msan/dtls_test.c | 4 ---
4 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h
index 504109e9d3f6f..8f3b71eb6ce74 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_interface.h
@@ -21,8 +21,8 @@ extern "C" {
SANITIZER_INTERFACE_ATTRIBUTE
uptr __sanitizer_get_estimated_allocated_size(uptr size);
SANITIZER_INTERFACE_ATTRIBUTE int __sanitizer_get_ownership(const void *p);
-SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE const void *
-__sanitizer_get_allocated_begin(const void *p);
+SANITIZER_INTERFACE_ATTRIBUTE const void *__sanitizer_get_allocated_begin(
+ const void *p);
SANITIZER_INTERFACE_ATTRIBUTE uptr
__sanitizer_get_allocated_size(const void *p);
SANITIZER_INTERFACE_ATTRIBUTE uptr __sanitizer_get_current_allocated_bytes();
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
index b13e2dc9e3327..252979f1c2baa 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.cpp
@@ -12,6 +12,7 @@
#include "sanitizer_tls_get_addr.h"
+#include "sanitizer_allocator_interface.h"
#include "sanitizer_atomic.h"
#include "sanitizer_flags.h"
#include "sanitizer_platform_interceptors.h"
@@ -26,13 +27,6 @@ struct TlsGetAddrParam {
uptr offset;
};
-// Glibc starting from 2.19 allocates tls using __signal_safe_memalign,
-// which has such header.
-struct Glibc_2_19_tls_header {
- uptr size;
- uptr start;
-};
-
// This must be static TLS
__attribute__((tls_model("initial-exec")))
static __thread DTLS dtls;
@@ -108,6 +102,14 @@ static const uptr kDtvOffset = 0x800;
static const uptr kDtvOffset = 0;
#endif
+extern "C" {
+SANITIZER_WEAK_ATTRIBUTE
+uptr __sanitizer_get_allocated_size(const void *p);
+
+SANITIZER_WEAK_ATTRIBUTE
+const void *__sanitizer_get_allocated_begin(const void *p);
+}
+
DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
uptr static_tls_begin, uptr static_tls_end) {
if (!common_flags()->intercept_tls_get_addr) return 0;
@@ -125,19 +127,18 @@ DTLS::DTV *DTLS_on_tls_get_addr(void *arg_void, void *res,
atomic_load(&number_of_live_dtls, memory_order_relaxed));
if (dtls.last_memalign_ptr == tls_beg) {
tls_size = dtls.last_memalign_size;
- VReport(2, "__tls_get_addr: glibc <=2.18 suspected; tls={0x%zx,0x%zx}\n",
+ VReport(2, "__tls_get_addr: glibc <=2.24 suspected; tls={0x%zx,0x%zx}\n",
tls_beg, tls_size);
} else if (tls_beg >= static_tls_begin && tls_beg < static_tls_end) {
// This is the static TLS block which was initialized / unpoisoned at thread
// creation.
VReport(2, "__tls_get_addr: static tls: 0x%zx\n", tls_beg);
tls_size = 0;
- } else if ((tls_beg % 4096) == sizeof(Glibc_2_19_tls_header)) {
- // We may want to check gnu_get_libc_version().
- Glibc_2_19_tls_header *header = (Glibc_2_19_tls_header *)tls_beg - 1;
- tls_size = header->size;
- tls_beg = header->start;
- VReport(2, "__tls_get_addr: glibc >=2.19 suspected; tls={0x%zx 0x%zx}\n",
+ } else if (const void *start =
+ __sanitizer_get_allocated_begin((void *)tls_beg)) {
+ tls_beg = (uptr)start;
+ tls_size = __sanitizer_get_allocated_size(start);
+ VReport(2, "__tls_get_addr: glibc >=2.25 suspected; tls={0x%zx,0x%zx}\n",
tls_beg, tls_size);
} else {
VReport(2, "__tls_get_addr: Can't guess glibc version\n");
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h
index a599c0bbc75cc..0ddab61deb102 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h
@@ -12,16 +12,24 @@
// the lack of interface that would tell us about the Dynamic TLS (DTLS).
// https://sourceware.org/bugzilla/show_bug.cgi?id=16291
//
-// The matters get worse because the glibc implementation changed between
-// 2.18 and 2.19:
-// https://groups.google.com/forum/#!topic/address-sanitizer/BfwYD8HMxTM
-//
-// Before 2.19, every DTLS chunk is allocated with __libc_memalign,
+// Before 2.25: every DTLS chunk is allocated with __libc_memalign,
// which we intercept and thus know where is the DTLS.
-// Since 2.19, DTLS chunks are allocated with __signal_safe_memalign,
-// which is an internal function that wraps a mmap call, neither of which
-// we can intercept. Luckily, __signal_safe_memalign has a simple parseable
-// header which we can use.
+//
+// Since 2.25: DTLS chunks are allocated with malloc. We could co-opt
+// the malloc interceptor to keep track of the last allocation, similar
+// to how we handle __libc_memalign; however, this adds some overhead
+// (since malloc, unlike __libc_memalign, is commonly called), and
+// requires care to avoid false negatives for LeakSanitizer.
+// Instead, we rely on our internal allocators - which keep track of all
+// its allocations - to determine if an address points to a malloc
+// allocation.
+//
+// There exists a since-deprecated version of Google's internal glibc fork
+// that used __signal_safe_memalign. DTLS_on_tls_get_addr relied on a
+// heuristic check (is the allocation 16 bytes from the start of a page
+// boundary?), which was sometimes erroneous:
+// https://bugs.chromium.org/p/chromium/issues/detail?id=1275223#c15
+// Since that check has no practical use anymore, we have removed it.
//
//===----------------------------------------------------------------------===//
diff --git a/compiler-rt/test/msan/dtls_test.c b/compiler-rt/test/msan/dtls_test.c
index 45c8fd38bf5f6..3c384256147a0 100644
--- a/compiler-rt/test/msan/dtls_test.c
+++ b/compiler-rt/test/msan/dtls_test.c
@@ -12,10 +12,6 @@
// Reports use-of-uninitialized-value, not analyzed
XFAIL: target={{.*netbsd.*}}
- // This is known to be broken with glibc-2.27+
- // https://bugs.llvm.org/show_bug.cgi?id=37804
- XFAIL: glibc-2.27
-
*/
#ifndef BUILD_SO
|
