php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78436 Missing addref in SplPriorityQueue EXTR_BOTH mode
Submitted: 2019-08-21 13:29 UTC Modified: 2019-08-23 13:23 UTC
From: zeriyoshi at gmail dot com Assigned: nikic (profile)
Status: Closed Package: *General Issues
PHP Version: 7.4.0beta2 OS: Linux
Private report: No CVE-ID: None
 [2019-08-21 13:29 UTC] zeriyoshi at gmail dot com
Description:
------------
In php 7.4 beta, if an object has a complex recursive reference, it will crash in the shutdown handler.

gdb backtrace:
(gdb) backtrace
#0  0x00005562abaa13c2 in zend_mm_alloc_small (heap=0x7f6f72800040, bin_num=11, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:1246
#1  0x00005562abaa1669 in zend_mm_alloc_heap (heap=0x7f6f72800040, size=120, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:1317
#2  0x00005562abaa44fc in _emalloc (size=88, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:2524
#3  0x00005562abb3ba54 in zend_objects_new (ce=0x7f6f72805e40)
    at /usr/src/php/Zend/zend_objects.c:196
#4  0x00005562abaed7ce in _object_and_properties_init (arg=0x7f6f72814520, 
    class_type=0x7f6f72805e40, properties=0x0) at /usr/src/php/Zend/zend_API.c:1397
#5  0x00005562abaed8ae in object_init_ex (arg=0x7f6f72814520, class_type=0x7f6f72805e40)
    at /usr/src/php/Zend/zend_API.c:1420
#6  0x00005562abb70bc5 in ZEND_NEW_SPEC_CONST_UNUSED_HANDLER ()
    at /usr/src/php/Zend/zend_vm_execute.h:9191
#7  0x00005562abbc9aef in execute_ex (ex=0x7f6f72814020)
--Type <RET> for more, q to quit, c to continue without paging--
   cute.h:54223
#8  0x00005562abbcd0e7 in zend_execute (op_array=0x7f6f72876600, return_value=0x0)
    at /usr/src/php/Zend/zend_vm_execute.h:57553
#9  0x00005562abae611c in zend_execute_scripts (type=8, retval=0x0, file_count=3)
    at /usr/src/php/Zend/zend.c:1663
#10 0x00005562aba2c8ec in php_execute_script (primary_file=0x7fff2f92c190)
    at /usr/src/php/main/main.c:2619
#11 0x00005562abbcfdc6 in do_cli (argc=2, argv=0x7f6f72e58d00)
    at /usr/src/php/sapi/cli/php_cli.c:962
#12 0x00005562abbd10e3 in main (argc=2, argv=0x7f6f72e58d00)
    at /usr/src/php/sapi/cli/php_cli.c:1352

Test script:
---------------
https://github.com/zeriyoshi/php74segv

Actual result:
--------------
(gdb) backtrace
#0  0x00005562abaa13c2 in zend_mm_alloc_small (heap=0x7f6f72800040, bin_num=11, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:1246
#1  0x00005562abaa1669 in zend_mm_alloc_heap (heap=0x7f6f72800040, size=120, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:1317
#2  0x00005562abaa44fc in _emalloc (size=88, 
    __zend_filename=0x5562ac358cb0 "/usr/src/php/Zend/zend_objects.c", 
    __zend_lineno=196, __zend_orig_filename=0x0, __zend_orig_lineno=0)
    at /usr/src/php/Zend/zend_alloc.c:2524
#3  0x00005562abb3ba54 in zend_objects_new (ce=0x7f6f72805e40)
    at /usr/src/php/Zend/zend_objects.c:196
#4  0x00005562abaed7ce in _object_and_properties_init (arg=0x7f6f72814520, 
    class_type=0x7f6f72805e40, properties=0x0) at /usr/src/php/Zend/zend_API.c:1397
#5  0x00005562abaed8ae in object_init_ex (arg=0x7f6f72814520, class_type=0x7f6f72805e40)
    at /usr/src/php/Zend/zend_API.c:1420
#6  0x00005562abb70bc5 in ZEND_NEW_SPEC_CONST_UNUSED_HANDLER ()
    at /usr/src/php/Zend/zend_vm_execute.h:9191
#7  0x00005562abbc9aef in execute_ex (ex=0x7f6f72814020)
--Type <RET> for more, q to quit, c to continue without paging--
   cute.h:54223
#8  0x00005562abbcd0e7 in zend_execute (op_array=0x7f6f72876600, return_value=0x0)
    at /usr/src/php/Zend/zend_vm_execute.h:57553
#9  0x00005562abae611c in zend_execute_scripts (type=8, retval=0x0, file_count=3)
    at /usr/src/php/Zend/zend.c:1663
#10 0x00005562aba2c8ec in php_execute_script (primary_file=0x7fff2f92c190)
    at /usr/src/php/main/main.c:2619
#11 0x00005562abbcfdc6 in do_cli (argc=2, argv=0x7f6f72e58d00)
    at /usr/src/php/sapi/cli/php_cli.c:962
#12 0x00005562abbd10e3 in main (argc=2, argv=0x7f6f72e58d00)
    at /usr/src/php/sapi/cli/php_cli.c:1352

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-08-21 13:34 UTC] nikic@php.net
-Status: Open +Status: Verified
 [2019-08-21 13:34 UTC] nikic@php.net
==11171== Invalid read of size 4
==11171==    at 0x92DDE6: zend_gc_delref (zend_types.h:1035)
==11171==    by 0x92E139: i_zval_ptr_dtor (zend_variables.h:43)
==11171==    by 0x92E35B: zval_ptr_dtor (zend_variables.c:84)
==11171==    by 0x6D0857: spl_ptr_heap_pqueue_elem_dtor (spl_heap.c:111)
==11171==    by 0x6D14FE: spl_ptr_heap_destroy (spl_heap.c:356)
==11171==    by 0x6D15A6: spl_heap_object_free_storage (spl_heap.c:377)
==11171==    by 0x98B49C: zend_objects_store_del (zend_objects_API.c:194)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)
==11171==    by 0x98357D: i_zval_ptr_dtor (zend_variables.h:44)
==11171==    by 0x983985: zend_object_std_dtor (zend_objects.c:70)
==11171==    by 0x98B49C: zend_objects_store_del (zend_objects_API.c:194)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)
==11171==  Address 0x12552bd0 is 0 bytes inside a block of size 56 free'd
==11171==    at 0x4C30D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11171==    by 0x8F70AF: _efree_custom (zend_alloc.c:2411)
==11171==    by 0x8F71F0: _efree (zend_alloc.c:2531)
==11171==    by 0x94A834: zend_array_destroy (zend_hash.c:1637)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)
==11171==    by 0x9453D3: i_zval_ptr_dtor (zend_variables.h:44)
==11171==    by 0x94A6FC: zend_array_destroy (zend_hash.c:1615)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)
==11171==    by 0x9453D3: i_zval_ptr_dtor (zend_variables.h:44)
==11171==    by 0x94A6CA: zend_array_destroy (zend_hash.c:1611)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)
==11171==    by 0x9453D3: i_zval_ptr_dtor (zend_variables.h:44)
==11171==  Block was alloc'd at
==11171==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11171==    by 0x8F80F0: __zend_malloc (zend_alloc.c:2961)
==11171==    by 0x8F7048: _malloc_custom (zend_alloc.c:2402)
==11171==    by 0x8F7176: _emalloc (zend_alloc.c:2521)
==11171==    by 0x945C94: _zend_new_array (zend_hash.c:256)
==11171==    by 0xA02FB1: ZEND_INIT_ARRAY_SPEC_CV_UNUSED_HANDLER (zend_vm_execute.h:46146)
==11171==    by 0xA0E9AA: execute_ex (zend_vm_execute.h:57221)
==11171==    by 0xA0EF21: zend_execute (zend_vm_execute.h:57553)
==11171==    by 0x9328BA: zend_execute_scripts (zend.c:1663)
==11171==    by 0x893110: php_execute_script (main.c:2619)
==11171==    by 0xA11B2D: do_cli (php_cli.c:962)
==11171==    by 0xA12C96: main (php_cli.c:1352)
==11171== 
php: /home/nikic/php-7.4/Zend/zend_types.h:1035: zend_gc_delref: Assertion `p->refcount > 0' failed.
==11171== 
==11171== Process terminating with default action of signal 6 (SIGABRT)
==11171==    at 0xA127E97: raise (raise.c:51)
==11171==    by 0xA129800: abort (abort.c:79)
==11171==    by 0xA119399: __assert_fail_base (assert.c:92)
==11171==    by 0xA119411: __assert_fail (assert.c:101)
==11171==    by 0x92DE0A: zend_gc_delref (zend_types.h:1035)
==11171==    by 0x92E139: i_zval_ptr_dtor (zend_variables.h:43)
==11171==    by 0x92E35B: zval_ptr_dtor (zend_variables.c:84)
==11171==    by 0x6D0857: spl_ptr_heap_pqueue_elem_dtor (spl_heap.c:111)
==11171==    by 0x6D14FE: spl_ptr_heap_destroy (spl_heap.c:356)
==11171==    by 0x6D15A6: spl_heap_object_free_storage (spl_heap.c:377)
==11171==    by 0x98B49C: zend_objects_store_del (zend_objects_API.c:194)
==11171==    by 0x92E1C6: rc_dtor_func (zend_variables.c:57)

Possibly a GC issue in spl_ptr_heap.
 [2019-08-21 13:45 UTC] nikic@php.net
-Status: Verified +Status: Analyzed
 [2019-08-21 13:45 UTC] nikic@php.net
I switched SplPriorityQueue to use a custom structure instead of an array to store elements -- unfortunately it means that we are exposing PTR zvals to GC, which it doesn't support.

Simple reproducer to get a leak:

<?php  
$pqueue = new SplPriorityQueue();
$pqueue->insert($pqueue, 1);

Probably some variation could also reproduce a segfault.

Either we need to use a separate buffer for GC, but it might make more sense to generate the ptr heap implementation to also support storing two zvals side by side, which will, as an additional bonus, also be more efficient.
 [2019-08-23 12:28 UTC] nikic@php.net
-Summary: SEGV in complicated recursive reference object. +Summary: Broken GC for SplPriorityQueue -Status: Analyzed +Status: Assigned -Assigned To: +Assigned To: nikic
 [2019-08-23 13:23 UTC] nikic@php.net
-Summary: Broken GC for SplPriorityQueue +Summary: Missing addref in SplPriorityQueue EXTR_BOTH mode
 [2019-08-23 13:23 UTC] nikic@php.net
Okay, while the GC issue also exists, what is actually causing the segfault here is something else, reproducible with just:

<?php
$pq = new SplPriorityQueue();
$pq->insert(new stdClass, 1);
var_dump($pq);
var_dump($pq);
 [2019-08-23 13:28 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=13e92223c0c102b33b775ff1e4f27bcca72f0eb9
Log: Fixed bug #78436
 [2019-08-23 13:28 UTC] nikic@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 15:01:29 2024 UTC