Skip to content

Don’t reset func in zend_closure_internal_handler - #9849

Merged
bwoebi merged 1 commit into
php:PHP-8.2from
rioderelfte:dont-clear-func-in-closure
Nov 2, 2022
Merged

bwoebi merged 1 commit into
php:PHP-8.2from
rioderelfte:dont-clear-func-in-closure

Conversation

@rioderelfte

Copy link
Copy Markdown
Contributor

The pointer is used in _zend_observe_fcall_begin().

The new test segfaulted before the change.

This change was already part of #9823 and I think the commit was missed by accident when merging that PR.

The pointer is used in _zend_observe_fcall_begin().
@bwoebi

bwoebi commented Nov 2, 2022

Copy link
Copy Markdown
Member

Oh. Yes, that was a mistake. Merged :-)

@cmb69

cmb69 commented Nov 2, 2022

Copy link
Copy Markdown
Member

Please don't forget to merge upwards next time (GH won't do this automatically) :)

@bwoebi

bwoebi commented Nov 2, 2022

Copy link
Copy Markdown
Member

Note: I initially did not merge that change because it was incorrect, then forgot about it and merged it too quickly today. Fixed that in 8e49d7f.

The main point is that you just swapped a NULL pointer by an use-after-free (the OBJ_RELEASE() in that line before the EX(func) = NULL may release the closure if it's in a temporary only).
The proper fix is delaying the freeing until after observers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants