Check the stack limit when calling internal functions#22545
Conversation
| #ifdef ZEND_CHECK_STACK_LIMIT | ||
| if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) { | ||
| zend_call_stack_size_error(); | ||
| zend_vm_stack_free_args(call); |
There was a problem hiding this comment.
Can we move this earlier? This would avoid all the cleanup code, and make maintenance easier
There was a problem hiding this comment.
Moved it ahead of zend_vm_stack_push_call_frame. With no frame, args, or fake_scope in play yet, the error path is just the error plus the fcall-cache release. It now also covers user calls, but those were already bounded by the VM's own check at their call opcodes, so there it just triggers a frame earlier.
zend_call_function invokes an internal callee's handler directly, with no VM frame and without the stack-limit check the interpreter runs at its call opcodes. An internal function that recurses through zend_call_function, such as a self- or mutually-attached SPL iterator, never yields back to the VM, so nothing bounds the recursion and the C stack overflows into a SEGV. Check zend_call_stack_overflowed() before pushing the call frame and raise the usual "Maximum call stack size reached" error on overflow. Running the check ahead of the frame setup keeps the error path free of teardown. Fixes phpGH-15672 Fixes phpGH-15911
iliaal
force-pushed
the
gh15672-stack-limit
branch
from
0355627 to
2219933
Compare
| GH-15672 (MultipleIterator attached to itself overflows the C stack) | ||
| --SKIPIF-- | ||
| <?php | ||
| if (!function_exists('zend_test_zend_call_stack_get')) die("skip zend_test_zend_call_stack_get() is not available"); |
There was a problem hiding this comment.
why is zend_test_zend_call_stack_get() needed for this test? Same with the zend_text extension being enabled.
also for the other test
There was a problem hiding this comment.
zend_test_zend_call_stack_get() is compiled only under #ifdef ZEND_CHECK_STACK_LIMIT, the same guard the fix's check sits behind, so function_exists() on it is the probe for whether stack-limit checking exists on this platform. Where it doesn't, EG(stack_limit) is never armed, the new check is a no-op, and the test recurses into a real SEGV instead of throwing. The probe lives in zend_test, hence the --EXTENSIONS-- line. Same skip guard as the existing Zend/tests/stack_limit/ tests.
3 participants
GH-16492 fixes GH-15911 by guarding AppendIterator against appending to itself; this is the general form. An internal function that recurses through zend_call_function (which invokes internal handlers directly, with no VM frame and no stack-limit check) never yields back to the VM, so the C stack overflows into a SEGV. This adds the interpreter's stack-limit check to that path, behind ZEND_CHECK_STACK_LIMIT, raising the usual "Maximum call stack size reached" error instead. It covers a MultipleIterator (GH-15672) or AppendIterator (GH-15911) attached to itself and a mutual cycle between two such iterators that a per-iterator guard cannot see. For a pure internal-to-internal cycle the error is uncatchable, since there is no user frame in the recursion to unwind into. Fixes #15672 and #15911.