Address possible use after free issue in HttpVCTable::remove_entry #5809
Conversation
|
[approve ci autest] |
|
[approve ci clang-analyzer] |
shinrich
changed the title
|
Yes, that is why we ran in in production before committing this. There doesn't seem to be a safe way to reach into the netvc at this point in the code. To be safe, any terminating netvc would have to be able to reach into this data structure and null out the relevant pointers. It seems that we are better off dealing with the stale events by tracking and canceling actions. Since PR #4020 was committed we also made a number of other fixes to track and cancel actions and to better lock and deliver events to the expected thread. |
zwoop
added
the
OnDocs
|
This does fix the ASAN leak, but as Bryan says, concerning that we might be missing something else that also should have been reverted? |
|
What say ye, should we land this? |
| if (e->read_vio != nullptr && e->read_vio->cont == sm) { | ||
| // Cleanup dangling i/o | ||
| if (e == sm->get_ua_entry() && sm->get_ua_txn() != nullptr) { | ||
| e->read_vio = sm->get_ua_txn()->do_io_read(nullptr, 0, nullptr); | ||
| } else if (e == sm->get_server_entry() && sm->get_server_session()) { | ||
| e->read_vio = sm->get_server_session()->do_io_read(nullptr, 0, nullptr); | ||
| } else { | ||
| ink_release_assert(false); | ||
| } | ||
| } | ||
| if (e->write_vio != nullptr && e->write_vio->cont == sm) { | ||
| // Cleanup dangling i/o | ||
| if (e == sm->get_ua_entry() && sm->get_ua_txn()) { | ||
| e->write_vio = sm->get_ua_txn()->do_io_write(nullptr, 0, nullptr); | ||
| } else if (e == sm->get_server_entry() && sm->get_server_session()) { | ||
| e->write_vio = sm->get_server_session()->do_io_write(nullptr, 0, nullptr); | ||
| } else { | ||
| ink_release_assert(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is cool, you can do multi-line comments now :).
|
Cherry-picked to v9.0.x branch. |
zwoop
removed
the
OnDocs
3 participants
Addressing issue #5807