Edit report at https://bugs.php.net/bug.php?id=69864&edit=1
ID: 69864
Updated by: ab@php.net
Reported by: james dot h dot cracknell at gmail dot com
Summary: Segfault in preg_replace_callback
Status: Verified
Type: Bug
Package: PCRE related
Operating System: Windows Server 2008 R2
PHP Version: 7.0.0alpha1
Assigned To: cmb
Block user comment: N
Private report: N
New Comment:
Hi Christoph,
nope, HashTable is unlikely to cause an issue. It's a very core API which is used everywhere,
so it's probably not causing this.
But if you're using the snippet from @james which is already know to cause crash, clear
there'll be such kinds of errors. So don't even need to mention that. And the cause of it,
as reported and also confirmed by yourself, is that the cache is freed without check which causes
accesses to the invalid memory. Except you've found out something new ;)
Thanks.
Previous Comments:
------------------------------------------------------------------------
[2015-06-19 13:40:36] cmb@php.net
Have you tried without the patch, Anatol? I get very similar
results before having applied the patch.
Nearly all of the warnings seem to be caused by
php_free_pcre_cache(). Storing non-zvals in a HashTable[1] might
not work reliably?
[1] <http://lxr.php.net/xref/PHP_TRUNK/ext/pcre/php_pcre.c#492>
------------------------------------------------------------------------
[2015-06-18 20:31:09] ab@php.net
I can see many errors like this with the new patch and the snippet @james posted earlier
==54386== Invalid read of size 1
==54386== at 0x409ED6D: ???
==54386== by 0xD801E77: ???
==54386== by 0xFFEFF3D8F: ???
==54386== Address 0xe699641 is 65 bytes inside a block of size 264 free'd
==54386== at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==54386== by 0x4E14A2: free_read_only_data (pcre_jit_compile.c:2139)
==54386== by 0x502C4A: _pcre_jit_free (pcre_jit_compile.c:10532)
==54386== by 0x4D311E: pcre_free_study (pcre_study.c:1672)
==54386== by 0x503C79: php_free_pcre_cache (php_pcre.c:96)
==54386== by 0xA3DF4C: _zend_hash_del_el_ex (zend_hash.c:935)
==54386== by 0xA3E032: _zend_hash_del_el (zend_hash.c:959)
==54386== by 0xA3F4DD: zend_hash_apply_with_argument (zend_hash.c:1463)
==54386== by 0x504D30: pcre_get_compiled_regex_cache (php_pcre.c:450)
==54386== by 0x5073D7: php_pcre_replace (php_pcre.c:1028)
==54386== by 0x508322: php_replace_in_subject (php_pcre.c:1361)
==54386== by 0x508976: preg_replace_impl (php_pcre.c:1422)
But in general - yep, in this case it's probably better to concentrate to fix master first, and
then to backport into 5.6 when we see it stable in master. Not sure what's wrong with
refcounts, probably better just to debug it.
Thanks.
------------------------------------------------------------------------
[2015-06-18 14:06:42] kalle@php.net
Fix assignee (was lost after Rasmus' comment)
------------------------------------------------------------------------
[2015-06-18 12:50:43] cmb@php.net
I still have not been able to get a crash; I've tested with
several 7.0.0alpha1 binaries and own VC 14 x64 builds. The tests
always exited cleanly, albeit with wrong results (either NULL or
"ba" instead of "bb").
Anyhow, I've attached an improved patch, including initialization
of the .refcount member, and additional refcounting for all paths
where the PCE might have to be prevented from being garbage
collected.
The patch is against master; for PHP 5.6 most notably the hack to
get the PCE at the beginning of pcre_clean_cache() has to be
changed. This raises a somewhat related issue wrt.
php_free_pcre_cache(). Line 92[1] does basically the same, but
assumes that data is a zval*. However, it is supposed to be a
pcre_cache_entry**[2]. That is currently not a real issue, as
.value is the first member of struct _zval_struct (and zend_value
is a union) but if the layout will change, Z_PTR_P(data) would
return garbage.
[1] <https://github.com/php/php-src/blob/8c8ad8f40ed9af2d95057a078dbaa844d072cb68/ext/pcre/php_pcre.c#L92>
[2] <https://github.com/php/php-src/blob/8c8ad8f40ed9af2d95057a078dbaa844d072cb68/ext/pcre/php_pcre.c#L492>
------------------------------------------------------------------------
[2015-06-18 12:49:36] cmb@php.net
The following patch has been added/updated:
Patch Name: pcre-refcount
Revision: 1434631776
URL: https://bugs.php.net/patch-display.php?bug=69864&patch=pcre-refcount&revision=1434631776
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=69864
--
Edit this bug report at https://bugs.php.net/bug.php?id=69864&edit=1