RFR: 8369393: NMT: poison the canaries of malloc header under ASAN build [v6]
Johan Sjölen
jsjolen at openjdk.org
Sun Oct 12 12:12:05 UTC 2025
On Fri, 10 Oct 2025 10:16:02 GMT, Afshin Zafari <azafari at openjdk.org> wrote:
>> NMT can detect malloc'd memory corruption using canary tests at header and footer of every memory region. This can only be done at free time of the memory where NNT checks the canaries and report error if they are not as expected.
>> In this PR, the canary parts also are poisoned using ASAN API to get notified whenever a read/write op is done. on the canary parts. `_size` member of the malloc header is also poisoned, since it is used for finding the footer address.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>
> gtest fix
We can have the unpoisoning as a RAII type for automatic scoping. With a RAII type for unpoisoning, maybe the code becomes so similar, that we can have one API for everything without ifdefing? The RAII type would unpoison more than it necessarily needs to to access the data of the method, but I think that's fine?
I don't get why the poisoning and unpoisoning of the header is done in the MallocTracker, that should probably be done in the constructor and some sort of destructor. For example, the `resolve_checked` in `MallocTracker::record_free_block` could be a different method, like `free_header` which returns the `FreeInfo` and marks the block as dead. The `FreeInfo` has the size, so this isn't a problem either.
Some sketching:
```c++
private:
// Better name??
struct AsanUnpoison {
bool _poison_initial_state;
MallocHeader& mh;
// TODO: Have this implementation be if-defd depending on ASAN inclusion
static set_posioned(MallocHeader& mh);
AsanUnpoison(MallocHeader& mh) : // Do the unpoisoning
~AsanUnpoison() // Do the poisoning
};
uint16_t get_footer() const {
AsanUnpoison aup(*this);
return build_footer(footer_address()[0], footer_address()[1]);
}
void set_footer(uint16_t v) {
AsanUnpoison aup(*this);
footer_address()[0] = (uint8_t)(v >> 8);
footer_address()[1] = (uint8_t)v;
}
public:
inline size_t() const {
AsanUnpoison aup(*this);
return _size;
}
What do you think? I think it might become neater like this.
-------------
PR Review: https://git.openjdk.org/jdk/pull/27685#pullrequestreview-3328327710
More information about the hotspot-runtime-dev
mailing list