From 03f0776d083b5bb570a8655e89c8fdc360d324a2 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 8 Jun 2024 22:00:19 +0100 Subject: [PATCH] Fix GH-13681: segfault when adding watchpoint fails. thus when removing its entry, no watch point is set and crash on pointer access. close GH-14513 --- NEWS | 3 +++ sapi/phpdbg/phpdbg_watch.c | 15 +++++++------ sapi/phpdbg/tests/gh13681.phpt | 39 ++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 sapi/phpdbg/tests/gh13681.phpt diff --git a/NEWS b/NEWS index 25e9351636e..a3d06d06066 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,9 @@ PHP NEWS - PDO ODBC: . Fixed bug GH-14367 (incompatible SDWORD type with iODBC). (Calvin Buckley) +- PHPDBG: + . Fixed bug GH-13681 (segfault on watchpoint addition failure). (David Carlier) + - Soap: . Fixed bug #47925 (PHPClient can't decompress response). (nielsdos) . Fix missing error restore code. (nielsdos) diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index 0468d4614fd..fef29a82938 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -315,7 +315,7 @@ void *phpdbg_watchpoint_userfaultfd_thread(void *phpdbg_globals) { struct uffd_msg fault_msg = {0}; while (read(globals->watch_userfaultfd, &fault_msg, sizeof(fault_msg)) == sizeof(fault_msg)) { - void *page = phpdbg_get_page_boundary((char *)(uintptr_t) fault_msg.arg.pagefault.address); + void *page = phpdbg_get_page_boundary((char *)(uintptr_t) fault_msg.arg.pagefault.address); zend_hash_index_add_empty_element(globals->watchlist_mem, (zend_ulong) page); struct uffdio_writeprotect unprotect = { .mode = 0, @@ -668,7 +668,7 @@ void phpdbg_watch_parent_ht(phpdbg_watch_element *element) { } void phpdbg_unwatch_parent_ht(phpdbg_watch_element *element) { - if (element->watch->type == WATCH_ON_BUCKET) { + if (element->watch && element->watch->type == WATCH_ON_BUCKET) { phpdbg_btree_result *res = phpdbg_btree_find(&PHPDBG_G(watch_HashTables), (zend_ulong) element->parent_container); ZEND_ASSERT(element->parent_container); if (res) { @@ -969,11 +969,14 @@ void phpdbg_remove_watchpoint(phpdbg_watchpoint_t *watch) { } void phpdbg_clean_watch_element(phpdbg_watch_element *element) { - HashTable *elements = &element->watch->elements; phpdbg_unwatch_parent_ht(element); - zend_hash_del(elements, element->str); - if (zend_hash_num_elements(elements) == 0) { - phpdbg_remove_watchpoint(element->watch); + + if (element->watch) { + HashTable *elements = &element->watch->elements; + zend_hash_del(elements, element->str); + if (zend_hash_num_elements(elements) == 0) { + phpdbg_remove_watchpoint(element->watch); + } } } diff --git a/sapi/phpdbg/tests/gh13681.phpt b/sapi/phpdbg/tests/gh13681.phpt new file mode 100644 index 00000000000..0ed79957c34 --- /dev/null +++ b/sapi/phpdbg/tests/gh13681.phpt @@ -0,0 +1,39 @@ +--TEST-- +phpdbg_watch null pointer access +--CREDITS-- +Yuancheng Jiang +--SKIPIF-- + +--FILE-- + 3, 1 => 4]; +?> +--PHPDBG-- +b 6 +r +w a $a +c +q +--EXPECTF-- +[Successful compilation of %s] +prompt> [Breakpoint #0 added at %s:%d] +prompt> *** Testing array_multisort() : Testing with anonymous arguments *** +bool(true) +Done +[Breakpoint #0 at %s:%d, hits: 1] +>00006: $a = []; + 00007: $a[0] = 1; + 00008: $a[0] = 2; +prompt> prompt> [Script ended normally] +prompt>