Skip to content

Fix GH-22759: Keep Io\Poll watcher registry in sync#22760

Open
LamentXU123 wants to merge 5 commits into
php:masterfrom
LamentXU123:fix-22759
Open

Fix GH-22759: Keep Io\Poll watcher registry in sync#22760
LamentXU123 wants to merge 5 commits into
php:masterfrom
LamentXU123:fix-22759

Conversation

@LamentXU123

@LamentXU123 LamentXU123 commented Jul 16, 2026

Copy link
Copy Markdown
Member

Also remove the watcher from the Context registry when it is explicitly removed. and roll back the backend registration on failure.

Fixed #22759

@devnexen

Copy link
Copy Markdown
Member

fix looks correct to me.

Comment thread ext/standard/io_poll.c Outdated

zend_ulong hash_key = php_io_poll_compute_ptr_key(handle);
zend_hash_index_add(intern->watchers, hash_key, &watcher_zv);
if (zend_hash_index_add(intern->watchers, hash_key, &watcher_zv) == NULL) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can zend_hash_index_add actually fail here? i.e. A duplicate handle hits PHP_POLL_ERR_EXISTS at line 751 first.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't. I wrote this as defensive stuff just in case but I don't spot the duplicate handle tho. Thanks.

@ndossche
ndossche removed their request for review July 17, 2026 16:59
@ndossche

Copy link
Copy Markdown
Member

I removed my request for review.
Tit for tat / trade offer: I review this if bukka reviews at least one of my Imagick fixes.

@LamentXU123
LamentXU123 requested review from arnaud-lb and iliaal July 17, 2026 17:14
Comment thread ext/standard/io_poll.c Outdated
Comment thread ext/standard/tests/poll/gh22759.phpt Outdated
@LamentXU123
LamentXU123 marked this pull request as draft July 17, 2026 17:53
@iliaal

iliaal commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

After last set of changes LGTM

@LamentXU123
LamentXU123 marked this pull request as ready for review July 17, 2026 18:14
@LamentXU123
LamentXU123 requested a review from arnaud-lb July 17, 2026 18:14
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.

Io\Poll use-after-free

5 participants