Skip to content

Fix GH-24430: Check for user frame before reading opline in generator dtor - #25278

Open
prateekbhujel wants to merge 1 commit into
php:PHP-8.4from
prateekbhujel:fix/gh24430-generator-dtor-opline-segv
Open

prateekbhujel wants to merge 1 commit into
php:PHP-8.4from
prateekbhujel:fix/gh24430-generator-dtor-opline-segv

Conversation

@prateekbhujel

Copy link
Copy Markdown
Contributor

Fixes GH-24430

When zend_generator_dtor_storage() executes a generator's finally block while EG(exception) is already set (for example, when an earlier generator's finally block threw during gc_collect_cycles() inside a fiber), EG(current_execute_data) is the gc_destructor_fiber() internal function frame.

Because zend_vm_init_call_frame() does not initialize call->opline for internal function frames, EG(current_execute_data)->opline contains uninitialized stack memory when the fiber VM stack page is allocated from dirty heap memory or under AddressSanitizer. Reading EG(current_execute_data)->opline->opcode without first checking EG(current_execute_data)->func && ZEND_USER_CODE(EG(current_execute_data)->func->common.type) then dereferences an uninitialized pointer.

Guard the opline check with EG(current_execute_data)->func && ZEND_USER_CODE(EG(current_execute_data)->func->common.type), matching is_handle_exception_set() in Zend/zend_exceptions.c.

cc @alexandre-daubois @iluuu1994

…tor dtor

When zend_generator_dtor_storage() runs finally blocks while an exception is already pending (such as when an earlier generator's finally block threw during gc_collect_cycles()), EG(current_execute_data) can be an internal function frame like gc_destructor_fiber() whose opline slot on the VM stack is uninitialized.

Check that EG(current_execute_data)->func is user code before dereferencing EG(current_execute_data)->opline->opcode, matching is_handle_exception_set().

Fixes phpGH-24430
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.

1 participant