Crash in ZEND_RETURN/GC/zend_call_function

[tstarling]

Description

I hit a segfault while running MediaWiki tests. I have not yet figured out how to make a short test case, but I was able to isolate the bug using gdb.

(gdb) bt
#0  0x000055555580cc23 in zend_call_function (fci=0x7fffffff9fa0, fci_cache=0x7fffffff9f80) at ./Zend/zend_execute_API.c:693
#1  0x000055555580d87d in zend_call_known_function (fn=fn@entry=0x7ffff14614b8, object=object@entry=0x7fffd16e6140, 
    called_scope=<optimized out>, retval_ptr=retval_ptr@entry=0x0, param_count=param_count@entry=0, params=params@entry=0x0, named_params=0x0)
    at ./Zend/zend_execute_API.c:985
#2  0x00005555558a08f4 in zend_call_known_instance_method (params=0x0, param_count=0, retval_ptr=0x0, object=0x7fffd16e6140, fn=0x7ffff14614b8)
    at ./Zend/zend_API.h:596
#3  zend_call_known_instance_method_with_0_params (retval_ptr=0x0, object=0x7fffd16e6140, fn=0x7ffff14614b8) at ./Zend/zend_API.h:596
#4  zend_objects_destroy_object (object=0x7fffd16e6140) at ./Zend/zend_objects.c:163
#5  0x0000555555893d90 in zend_gc_collect_cycles () at ./Zend/zend_gc.c:1549
#6  0x0000555555892b47 in gc_possible_root_when_full (ref=0x7fffd19cd2a0) at ./Zend/zend_gc.c:592
#7  0x000055555588341b in execute_ex (ex=0x7fffffff9fa0) at ./Zend/zend_vm_execute.h:58430
#8  0x000055555588660d in zend_execute (op_array=0x7ffff5489000, return_value=0x0) at ./Zend/zend_vm_execute.h:59499
#9  0x000055555581b0bd in zend_execute_scripts (type=-180232560, type@entry=8, retval=retval@entry=0x0, file_count=file_count@entry=3)
    at ./Zend/zend.c:1694
#10 0x00005555557b8a8a in php_execute_script (primary_file=<optimized out>) at ./main/main.c:2542
#11 0x00005555558acd18 in do_cli (argc=3, argv=0x555555a43410) at ./sapi/cli/php_cli.c:951
#12 0x000055555566c8b8 in main (argc=3, argv=0x555555a43410) at ./sapi/cli/php_cli.c:1339

The code at frame 7 is:

if (EXPECTED(!(EX_CALL_INFO() & (ZEND_CALL_CODE|ZEND_CALL_OBSERVED)))) {
	zend_refcounted *ref = Z_COUNTED_P(retval_ptr);
	ZVAL_COPY_VALUE(return_value, retval_ptr);
	if (GC_MAY_LEAK(ref)) {
		gc_possible_root(ref);
	}
	ZVAL_NULL(retval_ptr);
	break;

It fails to call SAVE_OPLINE() before invoking the GC. At frame 0 we have

} else if (EG(current_execute_data)->func &&
		   ZEND_USER_CODE(EG(current_execute_data)->func->common.type) &&
		   EG(current_execute_data)->opline->opcode != ZEND_DO_FCALL &&
		   EG(current_execute_data)->opline->opcode != ZEND_DO_ICALL &&
		   EG(current_execute_data)->opline->opcode != ZEND_DO_UCALL &&
		   EG(current_execute_data)->opline->opcode != ZEND_DO_FCALL_BY_NAME) {

but EG(current_execute_data)->opline is uninitialised garbage, so dereferencing it segfaults.

PHP Version

PHP 8.0.20

Operating System

No response

[tstarling]

@dstogov any thoughts? I can only see three calls to the GC from the VM, and only two of them lack SAVE_OPLINE(): one in ZEND_RETURN and another in ZEND_BIND_GLOBAL. I can make a patch that adds SAVE_OPLINE(), but usually you have your own ideas about how to fix this kind of thing.

[tstarling]

OK, no thoughts. My pull requests are getting more activity than this bug report, so I guess I will make a pull request.

[dstogov]

@tstarling thanks for detailed analyses and the patch. Sorry, I was on vacation and wasn't able to review this early. I'm not very happy, this is already committed, because this introduce slight performance penalty for very rare case. Anyway, thanks. I'll check if I can reduce overhead on top of this.

[dstogov]

After all, I think, the patch is fine and doesn't require any improvement.

[navisoft]

Hi @tstarling

Could you help take a look on this #9536, seems they relative each other, I have checked and ext/opcache/ZendAccelerator.c:2217 got the uninitialised garbage also

if (!EG(current_execute_data) || !EG(current_execute_data)->opline ||
			    !EG(current_execute_data)->func ||
			    !ZEND_USER_CODE(EG(current_execute_data)->func->common.type) ||
			    EG(current_execute_data)->opline->opcode != ZEND_INCLUDE_OR_EVAL ||
			    (EG(current_execute_data)->opline->extended_value != ZEND_INCLUDE_ONCE &&
			     EG(current_execute_data)->opline->extended_value != ZEND_REQUIRE_ONCE)) {

Thank you!

[dstogov]

@navisoft this is a different problem. Frames for internal function don't have to initialize opline. This should be fixed now.