You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
NULL-ptr dereference when using register_tick_function in destructor #18033
Results in a segmentation fault of the OSS-Fuzz harness php-fuzz-function-jit & php-fuzz-tracing-jit. It appears that the code snippet executes fine without jit, but crashes with the following stacktrace when jit is enabled:
#0 0x55bf976bdfc4 in zend_llist_apply /src/php-src/Zend/zend_llist.c:183
#1 0x55bf976be434 in zend_llist_apply_with_argument /src/php-src/Zend/zend_llist.c:236:3
#2 0x55bf97107be3 in php_run_ticks /src/php-src/main/php_ticks.c:68:2
#3 0x55bf974e3214 in ZEND_TICKS_SPEC_HANDLER /src/php-src/Zend/zend_vm_execute.h:3154:4
#4 0x55bf973e716b in execute_ex /src/php-src/Zend/zend_vm_execute.h:58595:7
#5 0x55bf973e7ba2 in zend_execute /src/php-src/Zend/zend_vm_execute.h:64247:2
#6 0x55bf9774e0f3 in fuzzer_do_request_from_buffer /src/php-src/sapi/fuzzer/fuzzer-sapi.c:274:5
#7 0x55bf9774c92e in LLVMFuzzerTestOneInput /src/php-src/sapi/fuzzer/fuzzer-tracing-jit.c:43:3
#8 0x55bf96a1f020 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
#9 0x55bf96a0a295 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
#10 0x55bf96a0fd2f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
#11 0x55bf96a3afd2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#12 0x7f271614b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 0702430aef5fa3dda43986563e9ffcc47efbd75e)
#13 0x55bf96a0247d in _start (/out/php-fuzz-tracing-jit+0x80247d)
We discovered this crash by fuzzing using the OSS-Fuzz infrastructure. We verified the issue to exist in commit 8731c95
This isn't JIT specific or anything. You can reproduce it outside of JIT by using the --repeat CLI option too.
The problem is that php_request_shutdown calls php_deactivate_ticks prior to running destructors and the shutdown functions. So if a destructor or shutdown function re-registers a tick function, then the user tick functions handler will be added back to PG(tick_functions). When the next request happens, the list PG(tick_functions) still contains an entry to call the user tick functions (added in the previous request during shutdown as I said). This causes a NULL deref eventually because run_user_tick_functions assumes that if it is called then BG(user_tick_functions) must be non-NULL (which is a sensible invariant).
We could move php_deactivate_ticks below zend_call_destructors in php_request_shutdown. But this is a behaviour change observable to userspace. Arguably the cleanest imo
We could keep the current php_deactivate_ticks and add another call below zend_call_destructors, this is no behaviour change but is ugly.
We could reset the tick function in the request startup, but this is also ugly and I don't like that it crosses request boundary in the first place.
I would consider the current behaviour a bug, as destructors should behave similarly when executed before or during shutdown. For this reason I prefer solution 1, at least in master.
Error handlers can also be executed during shutdown, so there shouldn't be (more) issues. (Only issue I can think of is that destructors of the tick function dependencies may have already been called, but this can also happen before shutdown.)
Activity
ndossche commentedon Mar 12, 2025
This isn't JIT specific or anything. You can reproduce it outside of JIT by using the
--repeatCLI option too.The problem is that
php_request_shutdowncallsphp_deactivate_ticksprior to running destructors and the shutdown functions. So if a destructor or shutdown function re-registers a tick function, then the user tick functions handler will be added back toPG(tick_functions). When the next request happens, the listPG(tick_functions)still contains an entry to call the user tick functions (added in the previous request during shutdown as I said). This causes a NULL deref eventually becauserun_user_tick_functionsassumes that if it is called thenBG(user_tick_functions)must be non-NULL (which is a sensible invariant).ndossche commentedon Mar 12, 2025
Some solution ideas:
php_deactivate_ticksbelowzend_call_destructorsinphp_request_shutdown. But this is a behaviour change observable to userspace. Arguably the cleanest imophp_deactivate_ticksand add another call belowzend_call_destructors, this is no behaviour change but is ugly.cc @iluuu1994 @arnaud-lb Suggestions on what you prefer?
bwoebi commentedon Mar 12, 2025
But I think I prefer 1.
arnaud-lb commentedon Mar 12, 2025
I would consider the current behaviour a bug, as destructors should behave similarly when executed before or during shutdown. For this reason I prefer solution 1, at least in master.
Error handlers can also be executed during shutdown, so there shouldn't be (more) issues. (Only issue I can think of is that destructors of the tick function dependencies may have already been called, but this can also happen before shutdown.)
iluuu1994 commentedon Mar 13, 2025
Solution 1. also sounds best to me. I'd prefer targetting
master, given nobody encountered this issue in a production system.Fix phpGH-18033: NULL-ptr dereference when using register_tick_functi…
Fix phpGH-18033: NULL-ptr dereference when using register_tick_functi…