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

[Thomas Stuefe]

On Mon, 10 Nov 2025 14:13:48 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> I see what you mean. 
>> 
>> Ugh. I forgot that we cannot selectively poison write access only but still allow read access, because otherwise poisoning the whole header would be much simpler and nicer. But unpoisoning every time one checks a header would be highly annoying.
>> 
>> In order to simplify, and not to pay too high a penalty on access if ASAN is enabled, I would consider:
>> - don't bother poisoning `_size`. This field is frequently accessed, as opposed to the `_canary` field. I think cases where we overwrite `_size` but *don't* overwrite either the byte before it (which should be guarded by ASAN anyway since it precedes the first valid byte) or the canary (which is now also poisoned and directly precedes the user payload) should be exceedingly rare. 
>> - I would just not bother checking NMT block integrity at all if ASAN is enabled. That should reduce the number of accesses to the `_canary` to zero, and since ASAN reports right away when the canary is hit, we will see it right away. Now you won't need unpoisoning on canary read.
>
> Wouldn't this lead to more branches in the code, and questions like "why is this poisoned, but not _size?" etc? I wouldn't call the `_size` field in the header frequently accessed (once at malloc, once at free, any I'm missing?), it's more than 0 I guess :-).
> 
> I haven't measured, so grains of salt and so on, but the cost of ASAN is already high, I don't think any additional cost to poisoning/unpoisoning NMT headers at malloc/free is going to add too much of a perf hit.

Hm, okay. I refreshed my memory, and yes, we only access the header on malloc and free. But then, what exactly is the purpose of selectively poisoning size and the canary, but leaving the rest unpoisoned? Either we must have perfect coverage - in which case we should poison the whole header - or we are happy with a 99% solution, in which case it's sufficient to poison only the canaries. Either way would be preferable to what the patch does now, which feels a bit arbitrary.

Also, since we only access the header on os::malloc/realloc/free, we can move up the RAII unpoisoning helper to those functions and reduce the number of invocations from 16 to three call sites.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2527077104