Bug #69864 [Ver]: Segfault in preg_replace_callback

Edit report at https://bugs.php.net/bug.php?id=69864&edit=1 ID: 69864 Updated by: cmb@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 +Assigned To: Block user comment: N Private report: N New Comment: My further findings regarding the invalid read (w/o the pcre-refcount patch): * all entries in the cache have been studied[1] (if that failed extra==NULL, so pcre_free_study won't be called) * it occurs only if entries are removed from the cache before shutdown[2] * if the cache is circumvented (by constructing a zval with the pce instead of calling zend_hash_update_mem[3] and freeing that zval at the end of php_pcre_replace[4]) the invalid reads may still happen * the valgrind backtrace is clear on where the respective memory block has been freed, but it doesn't show where the invalid read happens. Interestingly, frame #3 seems to be allocated on the stack. However, I'm not able to find what is causing the invalid read. :( [1] <https://github.com/php/php-src/blob/php-7.0.0alpha1/ext/pcre/php_pcre.c#L426> [2] <https://github.com/php/php-src/blob/php-7.0.0alpha1/ext/pcre/php_pcre.c#L448> [3] <https://github.com/php/php-src/blob/php-7.0.0alpha1/ext/pcre/php_pcre.c#L492> [4] <https://github.com/php/php-src/blob/php-7.0.0alpha1/ext/pcre/php_pcre.c#L1013> Previous Comments: ------------------------------------------------------------------------ [2015-06-20 15:46:25] ab@php.net Hi Christoph, great you stay on this. To me it's currently not clear whether it's not the same thing, so I would stay in the scope of this ticket therefore. But feel free to open another one, if you're sure. What I've discovered yet - pcre_free_study() is yeah, JIT related. Furthermore - regarding to the man page, it requires a pointer produced by pcre_study(). It's not always the case currently. When relying on this information, maybe some more could come out. I'd suggest just to investigate further till we come to the clear understanding about what's going on. Thanks. ------------------------------------------------------------------------ [2015-06-20 00:26:22] cmb@php.net Thanks for further investigation, Anatol, and the valuable hints. Indeed, the valgrind issues are not (directly) related to the patch. The following test script raises lots of these for current master (w/o the patch): <?php for ($i = 0; $i < 10000; $i++) { preg_replace('/foo'.$i.'bar/', 'baz', '???foo'.$i.'bar???'); } ?> All this depends on pcre.jit. If this (php.ini) configuration option is disabled, valgrind reports no problems. Also James' test script runs fine with the patch if pcre.jit is disabled. It seems to me that the valgrind/pcre.jit issue is not directly related to this ticket, and that it might be best to open a new ticket for it. I think it would be preferable to solve that issue before finally tackling this ticket. Anyhow, disregard my comments regarding storing PCE vs. zval in the HashTable. I had not read the code carefully enough. ------------------------------------------------------------------------ [2015-06-19 16:38:49] ab@php.net Christoph, debugged a bit w/o your patch. Consider these two lines from the trace pcre_study.c:1672 php_pcre.c:96 If you comment out the freeing of pce->extra, it'll pass but with memleaks. so that's not your patch crashing at all. So suspect some double free with pce->extras is the issue now. I think we should rethink the direction to go. Thanks. ------------------------------------------------------------------------ [2015-06-19 15:26:56] ab@php.net Ah, as from the patch, it should be pcre_cache_entry *pce = (pcre_cache_entry *) Z_PTR_P(data); but it probably won't fix the issue, just to mention. Thanks. ------------------------------------------------------------------------ [2015-06-19 14:51:49] ab@php.net i meant "using a crashy snippet without any patch" - so that's the plain situation where you see all the errors ... :) Thanks. ------------------------------------------------------------------------ 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