Fix MSAN use-after-free in Lua HeaderMapIterator destruction

[ashutosh321607]

Commit Message: Fix MSAN use-after-free in Lua HeaderMapIterator destruction
Additional Description: This change fixes a memory safety issue detected by MSAN where HeaderMapWrapper could hold a dangling reference to a HeaderMapIterator that was being garbage collected by Lua, or attempt to call markDead() on a partially destroyed iterator.
When HeaderMapIterator is destroyed by the Lua GC, it now explicitly notifies its parent HeaderMapWrapper. The wrapper then releases its reference to the iterator using a new resetWithoutMarkDead() method. This method avoids calling markDead() on the iterator, which is critical because invoking methods on an object currently executing its destructor is unsafe and triggers MSAN errors.
This ensures the HeaderMapWrapper's reference is cleanly nullified without accessing the dying object.
Risk Level: Low
Testing: Check with existing tests, none of the existing test failing
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

[repokitteh-read-only]

Hi @ashutosh321607, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #43204 was opened by ashutosh321607.

see: more, trace.

[ashutosh321607]

@kyessenov @AnirbanNandi

[wbpcode]

This change fixes a memory safety issue detected by MSAN where HeaderMapWrapper could hold a dangling reference to a HeaderMapIterator that was being garbage collected by Lua, or attempt to call markDead() on a partially destroyed iterator.

Sorry, but I didn't get this. Ideally, if the HeaderMapWrapper still keep the the iterator, than the iterator should not be destroyed?

[wbpcode]

When HeaderMapIterator is destroyed, the reference number of this object should be zero, then the parent_ should not hold this object in its iterator_ field?

Or in other word, if this object is destroyed, that means there no any LuaRef or LuaDeathRef keep this object, then should no one will call its markDead method?

[wbpcode]

I think this change is dangerous because the parent_ may be dangling reference if the parent HeaderMapWrapper object is destroyed first?

[ashutosh321607]

if the HeaderMapWrapper (parent) were destroyed before the HeaderMapIterator (child), the parent_ reference would become dangling, leading to undefined behavior when the iterator is eventually garbage collected.
To address this, I’ve refactored the implementation more a bit,

1. HeaderMapIterator now holds a HeaderMapWrapper* parent_ pointer instead of a reference.
2. I implemented onMarkDead() in HeaderMapIterator. When the parent releases the iterator (e.g., when the parent is destroyed), markDead() is called on the iterator. My override now sets parent_ = nullptr, effectively detaching the iterator.
3. In the HeaderMapIterator destructor, I now check if (parent_ != nullptr) before notifying the parent.

[wbpcode]

Thanks for the contribution.

[wbpcode]

/wait-any

[ashutosh321607]

This change fixes a memory safety issue detected by MSAN where HeaderMapWrapper could hold a dangling reference to a HeaderMapIterator that was being garbage collected by Lua, or attempt to call markDead() on a partially destroyed iterator.

Sorry, but I didn't get this. Ideally, if the HeaderMapWrapper still keep the the iterator, than the iterator should not be destroyed?

Basically LuaRef (used by HeaderMapWrapper) interacts with Lua's Garbage Collector.
LuaRef holds a registry reference to the Lua object. This prevents the object from being garbage collected during normal execution.
However, LuaRef does not prevent the object from being destroyed when the entire lua_State is being torn down (e.g., lua_close()). During teardown, Lua destroys all objects. If the HeaderMapIterator happens to be destroyed before the HeaderMapWrapper releases its reference (which can happen depending on the final GC order), the HeaderMapWrapper is left holding a reference to a now-dead object.
The MSAN error occurred specifically during this teardown phase:

1. lua_close() starts.
2. HeaderMapIterator is destroyed by Lua.
3. Later, HeaderMapWrapper (or its members) tries to access or clean up the reference to the iterator.
4. CRASH (Use-After-Free).

The fix ensures that when the HeaderMapIterator is destroyed in step 2, it explicitly tells the HeaderMapWrapper "I am gone," so the wrapper drops the reference immediately and safely.

[wbpcode]

cc @ashutosh321607

I think then the correct fix should be do cleanup (all every one's markDead) before call the lua_close(). And I believe we should do the clean up first before call the lua_close()?

Sorry for the nagging. I want to ensure we are in the correct direction. Ideally, we should add destructor to object the inherit from LuaBaseObject but use the markDead to manage the lifetime.