1
0
mirror of https://github.com/php/php-src.git synced 2026-03-24 00:02:20 +01:00

Fix GH-16727: Opcache bad signal 139 crash in ZTS bookworm (frankenphp)

Reproducer: https://github.com/php/php-src/issues/16727#issuecomment-2466256317

The root cause is a data race between two different threads:

1) We allocate a lower cased name for an anonymous class here:
   f97353f228/Zend/zend_compile.c (L8109)
2) This gets looked up as an interned string here:
   f97353f228/Zend/zend_compile.c (L8112)
   Assuming that there are uppercase symbols in the string and therefore
   `lcname != name` and that `lcname` is not yet in the interned string table,
   the pointer value of `lcname` won't change.
3) Here we add the string into the interned string table:
   f97353f228/Zend/zend_compile.c (L8223)
   However, in the meantime another thread could've added the string into the interned string table.
   This means that the following code will run, indirectly called via the `LITERAL_STR` macro,
   freeing `lcname`: 62e53e6f49/ext/opcache/ZendAccelerator.c (L572-L575)
4) In the reproducer we then access the freed `lcname` string here:
   f97353f228/Zend/zend_compile.c (L8229)

This is solved in my patch by retrieving the interned string pointer
and putting it in `lcname`.

Closes GH-16748.
This commit is contained in:
Niels Dossche
2024-11-10 23:22:53 +01:00
parent 1b379f5e55
commit 02ee521e20
2 changed files with 9 additions and 1 deletions

2
NEWS
View File

@@ -7,6 +7,8 @@ PHP NEWS
- Core:
. Fail early in *nix configuration build script. (hakre)
. Fixed bug GH-16727 (Opcache bad signal 139 crash in ZTS bookworm
(frankenphp)). (nielsdos)
- FPM:
. Fixed GH-16432 (PHP-FPM 8.2 SIGSEGV in fpm_get_status). (Jakub Zelenka)

View File

@@ -8043,7 +8043,13 @@ static void zend_compile_class_decl(znode *result, zend_ast *ast, bool toplevel)
}
opline->op1_type = IS_CONST;
LITERAL_STR(opline->op1, lcname);
/* It's possible that `lcname` is not an interned string because it was not yet in the interned string table.
* However, by this point another thread may have caused `lcname` to be added in the interned string table.
* This will cause `lcname` to get freed once it is found in the interned string table. If we were to use
* LITERAL_STR() here we would not change the `lcname` pointer to the new value, and it would point to the
* now-freed string. This will cause issues when we use `lcname` in the code below. We solve this by using
* zend_add_literal_string() which gives us the new value. */
opline->op1.constant = zend_add_literal_string(&lcname);
if (decl->flags & ZEND_ACC_ANON_CLASS) {
opline->opcode = ZEND_DECLARE_ANON_CLASS;