RFR: 8369393: NMT: poison the canaries of malloc header under ASAN build [v20]

[Paul Hübner]

On Mon, 10 Nov 2025 08:46:41 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:
> 
>   order of includes corrected.

Thanks for doing this, Afshin! I've left some very minor comments.

src/hotspot/share/nmt/mallocHeader.hpp line 120:

> 118:   void set_footer(uint16_t v) {
> 119:     AsanPoisoningHelper aph(reinterpret_cast<uint16_t*>(footer_address()));
> 120:     footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v;

Nit: can we please separate these onto two separate lines, it's cumbersome to read.

src/hotspot/share/nmt/mallocHeader.inline.hpp line 63:

> 61:   asan_unpoison_self();
> 62:   set_header_canary(_header_canary_dead_mark);
> 63:   NOT_LP64(set_alt_canary(_header_alt_canary_dead_mark);)

Question: is there a reason why the canaries and alt canaries are set here rather than in the unpoison function? While reviewing I had to jump back and forth between this & the other method to figure out what's going on.

src/hotspot/share/sanitizers/address.hpp line 86:

> 84:   AsanPoisoningHelper(const T* addr) : _memory_region(addr) {
> 85:     #if INCLUDE_ASAN
> 86:       ASAN_UNPOISON_MEMORY_REGION(_memory_region, sizeof(T));

So if I understand correctly, when we do e.g.:

AsanPoisoningHelper aph(reinterpret_cast<uint16_t*>(footer_address()));
footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v;

We are first unpoisoning, doing our memory modification, and then re-poisoning once the helper falls out of scope. Wouldn't it make that an `Asan*Un*poisoningHelper`? Maybe you could introduce a quick comment on what this class is/what you would use it for, because it's a bit counter intuitive at least to me.

test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp line 325:

> 323:     a = 2;
> 324:     EXPECT_EQ(a, 2);
> 325:   }

Maybe you could add another assert once the helper is out of scope to make sure that there is a death?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/27685#issuecomment-3511312859
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2510327606
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2510310739
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2510333720
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2510342573