Skip to content

Fix NULL dereference in zend_signal_handler_unblock#21253

Closed
Appla wants to merge 1 commit intophp:PHP-8.4from
Appla:signal-handler-segfault
Closed

Fix NULL dereference in zend_signal_handler_unblock#21253
Appla wants to merge 1 commit intophp:PHP-8.4from
Appla:signal-handler-segfault

Conversation

@Appla
Copy link
Copy Markdown
Contributor

@Appla Appla commented Feb 19, 2026

This PR attempts to Fixes #21252
Alternative fixes

@devnexen
Copy link
Copy Markdown
Member

php 8.3 is EOL, it s 8.4 minimum now

@Appla Appla changed the base branch from PHP-8.3 to PHP-8.4 February 19, 2026 11:28
@devnexen
Copy link
Copy Markdown
Member

I have more confidence in the 2nd fix personally. cc @bwoebi

@iluuu1994
Copy link
Copy Markdown
Member

iluuu1994 commented Mar 19, 2026

zend_signal_handler_unblock() is only called when there are pending signals. This is handled by ZEND_SIGNAL_UNBLOCK_INTERRUPTIONS():

# define ZEND_SIGNAL_UNBLOCK_INTERRUPTIONS() if (((SIGG(depth)--) == SIGG(blocked))) { zend_signal_handler_unblock(); }

(SIGG(depth)--) == SIGG(blocked) is true only when both SIGG(depth) and SIGG(blocked) are 1. SIGG(blocked) is set to 1 by zend_signal_handler_defer() when a signal is delayed.

zend_signal_handler_unblock() will SIG_BLOCK all incoming signals, which should prevent further signals from triggering, or being added to the queue. I don't see how this can cause a race condition.

Looking a bit at APCu, php_apc_finally looks suspicious, because it does not reset EG(bailout) during the execution of the finally block. This means that if a bailout is triggered, the finally block will be executed again. The analogous zend_catch in Zend/zend.h does have this code. This might or might not have something to do with it. I don't know if you have an easy way to test.

Edit: I created a PR here, as the code looks wrong regardless: krakjoe/apcu#611. I suppose you can just wait to see if this fixes your issue, once merged and released.

@iluuu1994
Copy link
Copy Markdown
Member

See #21252 (comment).

@iluuu1994 iluuu1994 closed this Mar 19, 2026
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.

3 participants