php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #66429 Logic flaw in handling of empty Interned strings
Submitted: 2014-01-06 23:27 UTC Modified: 2014-01-07 15:56 UTC
From: Terry at ellisons dot org dot uk Assigned:
Status: Closed Package: opcache
PHP Version: master-Git-2014-01-06 (Git) OS: N/A
Private report: No CVE-ID: None
 [2014-01-06 23:27 UTC] Terry at ellisons dot org dot uk
Description:
------------
The opcache extenstion turns off PHP string interning and substitutes its own strategy for collecting interned strings in its own SHM-based interned_strings hash during  in ZendAcclerator.c:zend_accel_init_shm().  It is then turned off in accel_shutdown().

Before and after these points in accel_startup() and accel_shutdown(), all interned strings created and dtored should lie between the original CG(interned_strings_start) and CG(interned_strings_end).  Note that this list of local interned strings is non-trivial (typically some 2,000) comprising interned_string representations of all builtin Zend and extension constants, function names and methods. I call these built-in interned strings.  

Between these same points all interned strings created and dtored should lie between ZCSG(interned_strings_start) and ZCSG(interned_strings_end).  I call these shared interned strings.

However, if a zval refering to a builtin interned string is dtored between accel_startup() and accel_shutdown(), then the !IS_INTERNED() macro will return false as the address of the string lies between original CG(interned_strings_start) and CG(interned_strings_end) and not between   
ZCSG(interned_strings_start) and ZCSG(interned_strings_end).  DTOR will then attempt to efree the string which is invalid as the string wasn't emalloc'ed.

As long as the scope and lifetimes of the two sets of interned strings are disjoint this all works fine.  However, one optimization introduced in PHP-5.6 has been missed and that is with strings and zvals created with the STR_EMPTY_ALLOC(), RETVAL_EMPTY_STRING() and ZVAL_EMPTY_STRING() macros.  These are all based on the CG(interned_empty_string) value and this is a built-in interned string.  If any of these are DTORed then the dtor will fail as described above.  ZendAccelerator should establish its own ZCSG(interned_empty_string) value to prevent this happening.


Test script:
---------------
php56 -r '' 

(with opcache enabled)


Patches

interned_empty_string.patch (last revision 2014-01-06 23:27 UTC by Terry at ellisons dot org dot uk)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-01-07 00:07 UTC] Terry at ellisons dot org dot uk
Sorry, I've just tested this on 5.5 and realised that I need 

#if ZEND_EXTENSION_API_NO >= PHP_5_5_X_API_NO
...
#endif 

around my references to interned_empty_string.  I'll post an updated patch tomorrow.
 [2014-01-07 15:56 UTC] Terry at ellisons dot org dot uk
-Status: Open +Status: Closed
 [2014-01-07 15:56 UTC] Terry at ellisons dot org dot uk
Sorry, I've just realised that I've missed some PHP-5.6 changes in ext/opcache -- including this specific fix.  I suppose that at least I came up with the same change after some blundering around ;-(

As a general note (that I also made 
As I commented in https://github.com/zendtech/ZendOptimizerPlus/issues/157, the code bases for ext/opcache and http://pecl.php.net/package/ZendOpcache should be identical, and hence these PHP-5.6-specifc changes to ext/opcache should be guarded by a #if ZEND_EXTENSION_API_NO > PHP_5_5_X_API_NO test and they currently aren't.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Sun Aug 25 05:01:26 2019 UTC