bahman/scylla
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

lsa: make sure align_up_for_asan() doesn't cause reads past end of segment

Browse Source 
In debug mode the LSA needs objects to be 8-byte aligned in order to
maximise coverage from the AddressSanitizer.

Usually `close_active()` creates a dummy objects that covers the end of
the segment being closed. However, it the last real objects ends in the
last eight bytes of the segment then that dummy won't be created because
of the alignment requirements. This broke exit conditions on loops
trying to read all objects in the segment and caused them to attempt to
dereference address at the end of the segment. This patch fixes that.

Fixes #4653.
This commit is contained in:
Paweł Dziepak
2019-07-10 18:19:24 +01:00
committed by Tomasz Grabiec
parent e32bdb6b90
commit eb7d17e5c5
1 changed files with 10 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
utils/logalloc.cc
View File

@@ -60,8 +60,8 @@ namespace debug {
constexpr size_t logalloc_alignment = 8;
}
template<typename T>
static void align_up_for_asan(T& val) {
val = align_up(val, size_t(8));
[[nodiscard]] static T align_up_for_asan(T val) {
return align_up(val, size_t(8));
}
template<typename T>
void poison(const T* addr, size_t size) {
@@ -74,8 +74,7 @@ void poison(const T* addr, size_t size) {
// * end of segment
// In all cases, we can align up the size to guarantee that asan
// is able to poison this.
align_up_for_asan(size);
ASAN_POISON_MEMORY_REGION(addr, size);
ASAN_POISON_MEMORY_REGION(addr, align_up_for_asan(size));
}
void unpoison(const char *addr, size_t size) {
ASAN_UNPOISON_MEMORY_REGION(addr, size);
@@ -85,7 +84,7 @@ namespace debug {
constexpr size_t logalloc_alignment = 1;
}
template<typename T>
static void align_up_for_asan(T& val) { }
[[nodiscard]] static T align_up_for_asan(T val) { return val; }
template<typename T>
void poison(const T* addr, size_t size) { }
void unpoison(const char *addr, size_t size) { }
@@ -1171,8 +1170,7 @@ private:
auto desc = object_descriptor(migrator);
auto desc_encoded_size = desc.encoded_size();
size_t obj_offset = align_up(_active_offset + desc_encoded_size, alignment);
align_up_for_asan(obj_offset);
size_t obj_offset = align_up_for_asan(align_up(_active_offset + desc_encoded_size, alignment));
if (obj_offset + size > segment::size) {
close_and_open();
return alloc_small(migrator, size, alignment);
@@ -1186,7 +1184,7 @@ private:
_active_offset = obj_offset + size;
// Align the end of the value so that the next descriptor is aligned
align_up_for_asan(_active_offset);
_active_offset = align_up_for_asan(_active_offset);
_active->record_alloc(_active_offset - old_active_offset);
return pos;
}
@@ -1197,9 +1195,8 @@ private:
static_assert(std::is_same<void, std::result_of_t<Func(const object_descriptor*, void*)>>::value, "bad Func signature");
auto pos = seg->at<const char>(0);
auto pos = align_up_for_asan(seg->at<const char>(0));
while (pos < seg->at<const char>(segment::size)) {
align_up_for_asan(pos);
auto old_pos = pos;
const auto desc = object_descriptor::decode_forwards(pos);
if (desc.is_live()) {
@@ -1209,6 +1206,7 @@ private:
} else {
pos = old_pos + desc.dead_size();
}
pos = align_up_for_asan(pos);
}
}
@@ -1448,8 +1446,7 @@ public:
auto pos = reinterpret_cast<const char*>(obj);
auto old_pos = pos;
auto desc = object_descriptor::decode_backwards(pos);
auto dead_size = size + (old_pos - pos);
align_up_for_asan(dead_size);
auto dead_size = align_up_for_asan(size + (old_pos - pos));
desc = object_descriptor::make_dead(dead_size);
auto npos = const_cast<char*>(pos);
desc.encode(npos);
@@ -1570,7 +1567,6 @@ public:
size_t offset = 0;
while (offset < segment_size) {
align_up_for_asan(offset);
auto pos = src->at<const char>(offset);
auto dpos = dst->at<char>(offset);
auto old_pos = pos;
@@ -1589,6 +1585,7 @@ public:
} else {
offset += desc.dead_size();
}
offset = align_up_for_asan(offset);
}
shard_segment_pool.on_segment_migration();
}