Skip to content

Commit

Permalink
Remove canaries
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdksjolen
jdksjolen committed Oct 17, 2024
1 parent d636e0d commit a3203f7
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 361 deletions.
54 changes: 10 additions & 44 deletions src/hotspot/share/nmt/mallocHeader.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,11 @@ class outputStream;
* If NMT is active (state >= minimal), we need to track allocations. A simple and cheap way to
* do this is by using malloc headers.
*
* The user allocation is preceded by a header and is immediately followed by a (possibly unaligned)
* footer canary:
*
* +--------------+------------- .... ------------------+-----+
* | header | user | can |
* | | allocation | ary |
* +--------------+------------- .... ------------------+-----+
* 16 bytes user size 2 byte
* +--------------+------------- .... ------------------+
* | header | user |
* | | allocation |
* +--------------+------------- .... ------------------+
* 16 bytes user size
*
* Alignment:
*
Expand All @@ -63,28 +60,20 @@ class outputStream;
*
* 8 9 10 11 12 13 14 15 16 ++
* +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------
* ... | malloc site table marker | flags | unused | canary | ... User payload ....
* ... | malloc site table marker | flags | unused | ... User payload ....
* +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------
*
* Layout on 32-bit:
*
* 0 1 2 3 4 5 6 7
* +--------+--------+--------+--------+--------+--------+--------+--------+
* | alt. canary | 32-bit size | ...
* | 32-bit size | ...
* +--------+--------+--------+--------+--------+--------+--------+--------+
*
* 8 9 10 11 12 13 14 15 16 ++
* +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------
* ... | malloc site table marker | flags | unused | canary | ... User payload ....
* ... | malloc site table marker | flags | unused | ... User payload ....
* +--------+--------+--------+--------+--------+--------+--------+--------+ ------------------------
*
* Notes:
* - We have a canary in the two bytes directly preceding the user payload. That allows us to
* catch negative buffer overflows.
* - On 32-bit, due to the smaller size_t, we have some bits to spare. So we also have a second
* canary at the very start of the malloc header (generously sized 32 bits).
* - The footer canary consists of two bytes. Since the footer location may be unaligned to 16 bits,
* the bytes are stored individually.
*/

class MallocHeader {
Expand All @@ -93,27 +82,13 @@ class MallocHeader {
const size_t _size;
const uint32_t _mst_marker;
const MemTag _mem_tag;
const uint8_t _unused;
uint16_t _canary;

static const uint16_t _header_canary_live_mark = 0xE99E;
static const uint16_t _header_canary_dead_mark = 0xD99D;
static const uint16_t _footer_canary_live_mark = 0xE88E;
static const uint16_t _footer_canary_dead_mark = 0xD88D;
NOT_LP64(static const uint32_t _header_alt_canary_live_mark = 0xE99EE99E;)
NOT_LP64(static const uint32_t _header_alt_canary_dead_mark = 0xD88DD88D;)
const uint8_t _unused[3];

// We discount sizes larger than these
static const size_t max_reasonable_malloc_size = LP64_ONLY(256 * G) NOT_LP64(3500 * M);

void print_block_on_error(outputStream* st, address bad_address) const;

static uint16_t build_footer(uint8_t b1, uint8_t b2) { return (uint16_t)(((uint16_t)b1 << 8) | (uint16_t)b2); }

uint8_t* footer_address() const { return ((address)this) + sizeof(MallocHeader) + _size; }
uint16_t get_footer() const { return build_footer(footer_address()[0], footer_address()[1]); }
void set_footer(uint16_t v) { footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v; }

template<typename InTypeParam, typename OutTypeParam>
inline static OutTypeParam resolve_checked_impl(InTypeParam memblock);

Expand All @@ -127,7 +102,7 @@ class MallocHeader {

inline MallocHeader(size_t size, MemTag mem_tag, uint32_t mst_marker);

inline static size_t malloc_overhead() { return sizeof(MallocHeader) + sizeof(uint16_t); }
inline static size_t malloc_overhead() { return sizeof(MallocHeader); }
inline size_t size() const { return _size; }
inline MemTag mem_tag() const { return _mem_tag; }
inline uint32_t mst_marker() const { return _mst_marker; }
Expand All @@ -136,15 +111,6 @@ class MallocHeader {
FreeInfo free_info() {
return FreeInfo{this->size(), this->mem_tag(), this->mst_marker()};
}
inline void mark_block_as_dead();
inline void revive();


bool is_dead() const { return _canary == _header_canary_dead_mark; }
bool is_live() const { return _canary == _header_canary_live_mark; }

// Used for debugging purposes only. Check header if it could constitute a valid (live or dead) header.
inline bool looks_valid() const;

// If block is broken, fill in a short descriptive text in out,
// an option pointer to the corruption in p_corruption, and return false.
Expand Down
55 changes: 1 addition & 54 deletions src/hotspot/share/nmt/mallocHeader.inline.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,11 @@

inline MallocHeader::MallocHeader(size_t size, MemTag mem_tag, uint32_t mst_marker)
: _size(size), _mst_marker(mst_marker), _mem_tag(mem_tag),
_unused(0), _canary(_header_canary_live_mark)
_unused()
{
assert(size < max_reasonable_malloc_size, "Too large allocation size?");
// On 32-bit we have some bits more, use them for a second canary
// guarding the start of the header.
NOT_LP64(_alt_canary = _header_alt_canary_live_mark;)
set_footer(_footer_canary_live_mark); // set after initializing _size
}

inline void MallocHeader::revive() {
assert(_canary == _header_canary_dead_mark, "must be dead");
assert(get_footer() == _footer_canary_dead_mark, "must be dead");
NOT_LP64(assert(_alt_canary == _header_alt_canary_dead_mark, "must be dead"));
_canary = _header_canary_live_mark;
NOT_LP64(_alt_canary = _header_alt_canary_live_mark);
set_footer(_footer_canary_live_mark);
}

// The effects of this method must be reversible with MallocHeader::revive()
inline void MallocHeader::mark_block_as_dead() {
_canary = _header_canary_dead_mark;
NOT_LP64(_alt_canary = _header_alt_canary_dead_mark);
set_footer(_footer_canary_dead_mark);
}

inline bool MallocHeader::is_valid_malloced_pointer(const void* payload, char* msg, size_t msglen) {
// Handle the pointer as an integral type
Expand Down Expand Up @@ -116,50 +97,16 @@ inline const MallocHeader* MallocHeader::resolve_checked(const void* memblock) {
return MallocHeader::resolve_checked_impl<const void*, const MallocHeader*>(memblock);
}


// Used for debugging purposes only. Check header if it could constitute a valid (live or dead) header.
inline bool MallocHeader::looks_valid() const {
// Note: we define these restrictions loose enough to also catch moderately corrupted blocks.
// E.g. we don't check footer canary.
return ( (_canary == _header_canary_live_mark NOT_LP64(&& _alt_canary == _header_alt_canary_live_mark)) ||
(_canary == _header_canary_dead_mark NOT_LP64(&& _alt_canary == _header_alt_canary_dead_mark)) ) &&
_size > 0 && _size < max_reasonable_malloc_size;
}

inline bool MallocHeader::check_block_integrity(char* msg, size_t msglen, address* p_corruption) const {
// Note: if you modify the error messages here, make sure you
// adapt the associated gtests too.

// Check header canary
if (_canary != _header_canary_live_mark) {
*p_corruption = (address)this;
jio_snprintf(msg, msglen, "header canary broken");
return false;
}

#ifndef _LP64
// On 32-bit we have a second canary, check that one too.
if (_alt_canary != _header_alt_canary_live_mark) {
*p_corruption = (address)this;
jio_snprintf(msg, msglen, "header canary broken");
return false;
}
#endif

// Does block size seems reasonable?
if (_size >= max_reasonable_malloc_size) {
*p_corruption = (address)this;
jio_snprintf(msg, msglen, "header looks invalid (weirdly large block size)");
return false;
}

// Check footer canary
if (get_footer() != _footer_canary_live_mark) {
*p_corruption = footer_address();
jio_snprintf(msg, msglen, "footer canary broken at " PTR_FORMAT " (buffer overflow?)",
p2i(footer_address()));
return false;
}
return true;
}

Expand Down
95 changes: 0 additions & 95 deletions src/hotspot/share/nmt/mallocTracker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ void* MallocTracker::record_free_block(void* memblock) {

deaccount(header->free_info());

header->mark_block_as_dead();

return (void*)header;
}

Expand All @@ -218,96 +216,3 @@ void MallocTracker::deaccount(MallocHeader::FreeInfo free_info) {
MallocSiteTable::deallocation_at(free_info.size, free_info.mst_marker);
}
}

// Given a pointer, look for the containing malloc block.
// Print the block. Note that since there is very low risk of memory looking
// accidentally like a valid malloc block header (canaries and all) so this is not
// totally failproof and may give a wrong answer. It is safe in that it will never
// crash, even when encountering unmapped memory.
bool MallocTracker::print_pointer_information(const void* p, outputStream* st) {
assert(MemTracker::enabled(), "NMT not enabled");

#if !INCLUDE_ASAN

address addr = (address)p;

// Carefully feel your way upwards and try to find a malloc header. Then check if
// we are within the block.
// We give preference to found live blocks; but if no live block had been found,
// but the pointer points into remnants of a dead block, print that instead.
const MallocHeader* likely_dead_block = nullptr;
const MallocHeader* likely_live_block = nullptr;
{
const size_t smallest_possible_alignment = sizeof(void*);
const uint8_t* here = align_down(addr, smallest_possible_alignment);
const uint8_t* const end = here - (0x1000 + sizeof(MallocHeader)); // stop searching after 4k
for (; here >= end; here -= smallest_possible_alignment) {
// JDK-8306561: cast to a MallocHeader needs to guarantee it can reside in readable memory
if (!os::is_readable_range(here, here + sizeof(MallocHeader))) {
// Probably OOB, give up
break;
}
const MallocHeader* const candidate = (const MallocHeader*)here;
if (!candidate->looks_valid()) {
// This is definitely not a header, go on to the next candidate.
continue;
}

// fudge factor:
// We don't report blocks for which p is clearly outside of. That would cause us to return true and possibly prevent
// subsequent tests of p, see os::print_location(). But if p is just outside of the found block, this may be a
// narrow oob error and we'd like to know that.
const int fudge = 8;
const address start_block = (address)candidate;
const address start_payload = (address)(candidate + 1);
const address end_payload = start_payload + candidate->size();
const address end_payload_plus_fudge = end_payload + fudge;
if (addr >= start_block && addr < end_payload_plus_fudge) {
// We found a block the pointer is pointing into, or almost into.
// If its a live block, we have our info. If its a dead block, we still
// may be within the borders of a larger live block we have not found yet -
// continue search.
if (candidate->is_live()) {
likely_live_block = candidate;
break;
} else {
likely_dead_block = candidate;
continue;
}
}
}
}

// If we've found a reasonable candidate. Print the info.
const MallocHeader* block = likely_live_block != nullptr ? likely_live_block : likely_dead_block;
if (block != nullptr) {
const char* where = nullptr;
const address start_block = (address)block;
const address start_payload = (address)(block + 1);
const address end_payload = start_payload + block->size();
if (addr < start_payload) {
where = "into header of";
} else if (addr < end_payload) {
where = "into";
} else {
where = "just outside of";
}
st->print_cr(PTR_FORMAT " %s %s malloced block starting at " PTR_FORMAT ", size " SIZE_FORMAT ", tag %s",
p2i(p), where,
(block->is_dead() ? "dead" : "live"),
p2i(block + 1), // lets print the payload start, not the header
block->size(), NMTUtil::tag_to_enum_name(block->mem_tag()));
if (MemTracker::tracking_level() == NMT_detail) {
NativeCallStack ncs;
if (MallocSiteTable::access_stack(ncs, *block)) {
ncs.print_on(st);
st->cr();
}
}
return true;
}

#endif // !INCLUDE_ASAN

return false;
}
4 changes: 1 addition & 3 deletions src/hotspot/share/nmt/memTracker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ void MemTracker::final_report(outputStream* output) {
// Given an unknown pointer, check if it points into a known region; print region if found
// and return true; false if not found.
bool MemTracker::print_containing_region(const void* p, outputStream* out) {
return enabled() &&
(MallocTracker::print_pointer_information(p, out) ||
VirtualMemoryTracker::print_containing_region(p, out));
return enabled() && VirtualMemoryTracker::print_containing_region(p, out);
}

void MemTracker::report(bool summary_only, outputStream* output, size_t scale) {
Expand Down
4 changes: 0 additions & 4 deletions src/hotspot/share/runtime/os.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -727,15 +727,11 @@ void* os::realloc(void *memblock, size_t size, MemTag mem_tag, const NativeCallS
NMTUtil::tag_to_name(mem_tag), NMTUtil::tag_to_name(header->mem_tag()));
const MallocHeader::FreeInfo free_info = header->free_info();

header->mark_block_as_dead();

// the real realloc
ALLOW_C_FUNCTION(::realloc, void* const new_outer_ptr = ::realloc(header, new_outer_size);)

if (new_outer_ptr == nullptr) {
// realloc(3) failed and the block still exists.
// We have however marked it as dead, revert this change.
header->revive();
return nullptr;
}
// realloc(3) succeeded, variable header now points to invalid memory and we need to deaccount the old block.
Expand Down
Loading

0 comments on commit a3203f7

Please sign in to comment.