[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Crash in ZEND_RETURN/GC/zend_call_function #9323

Closed
tstarling opened this issue Aug 13, 2022 · 6 comments
Closed

Crash in ZEND_RETURN/GC/zend_call_function #9323

tstarling opened this issue Aug 13, 2022 · 6 comments
Labels
Bug Status: Verified

Comments

@tstarling
Copy link
Contributor

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 tstarling mentioned this issue Aug 15, 2022
Merged
@tstarling
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

tstarling added a commit to tstarling/php-src that referenced this issue Aug 19, 2022
@tstarling
Fix crash when the VM enters userspace code via the GC
ef3f40b 
Closes phpGH-9323
@tstarling tstarling mentioned this issue Aug 19, 2022
Closed
@dstogov
Copy link
Member
dstogov commented Aug 22, 2022

@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
Copy link
Member
dstogov commented Aug 22, 2022

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

@navisoft
Copy link

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!

@tstarling tstarling mentioned this issue Sep 17, 2022
Open
@dstogov
Copy link
Member
dstogov commented Sep 19, 2022

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

@chrillep chrillep mentioned this issue Oct 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Verified
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@tstarling @dstogov @navisoft @damianwadley and others