Fix GH-21824: Avoid unserialize context reuse during cleanup#21828
Fix GH-21824: Avoid unserialize context reuse during cleanup#21828prateekbhujel wants to merge 1 commit intophp:masterfrom
Conversation
|
Thanks for the PR. Please target master, this is a largely artificial issue. |
|
Makes sense. I’ll retarget it to master. I started with PHP-8.4 because it is still a crash, but agreed, this is artificial enough that master is the better target. |
7b395e7 to
2146eef
Compare
|
This old PR from Nikita seems potentially related: #5039 |
|
Yeah, I took a look at #5039. That one seems broader than this fix. It reworks the serialization/unserialization locking model more generally, while the issue here is the outermost destroy path still leaving BG(unserialize) pointing at a context while var_destroy() can run destructors. So I kept this one narrow and just clear the outer unserialize context before that cleanup runs, so destructor reentry starts with fresh state instead of reusing the one currently being torn down. |
Fixes GH-21824.
The outer unserialize context stayed in
BG(unserialize)whilephp_unserialize_datawas being destroyed. If an object destructor re-enteredunserialize()from that cleanup path, the nested call could reuse the outer context after its extra var entries had already been freed.Clear the outer context before running
var_destroy()for the outermost call, so destructor reentry starts with a fresh unserialize context instead of the one currently being torn down.Tests:
sapi/cli/php run-tests.php -q ext/standard/tests/serialize/gh21824.phptsapi/cli/php run-tests.php -q ext/standard/tests/serialize