Skip to content

Commit 9056908

Browse files
morrisonlevimorrisonlevi
authored
fix GH-20469: unsafe inheritance cache replay with reentrant autoloading (#22221)
Inheritance cache dependencies are collected while a class is being linked. During delayed variance resolution, autoloading can re-enter class linking and use the current class while it is only nearly linked. If that class is persisted in the inheritance cache, a later request can replay dependencies in a different order and observe an incomplete hierarchy. When delayed autoloading causes the class to be used through the unlinked/nearly-linked lookup path, mark it as non-cacheable after load_delayed_classes() returns. This also catches cases where the class's variance obligations were resolved reentrantly, before the direct resolve_delayed_variance_obligations() call would run. If dependency tracking already allocated a temporary dependency table, free it when cache insertion is skipped. Restrict this cleanup to classes that entered inheritance-cache construction, because otherwise inheritance_cache is not a dependency table and may contain unrelated or uninitialized data. This preserves inheritance-cache use for delayed-variance classes that did not participate in this reentrant cycle. With the invalid cache entry prevented, unlinked_instanceof() can keep using instanceof_function() for linked classes.
1 parent 2210fda commit 9056908

6 files changed

Lines changed: 492 additions & 0 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ PHP NEWS
4444
. Fix stmt->query leak in mysqli_execute_query() validation errors.
4545
(David Carlier)
4646

47+
- Opcache:
48+
. Fixed bug GH-20469 (Unsafe inheritance cache replay with reentrant
49+
autoloading). (Levi Morrison)
50+
4751
- Phar:
4852
. Fixed a bypass of the magic ".phar" directory protection in
4953
Phar::addEmptyDir() for paths starting with "/.phar", while allowing

‎Zend/zend_inheritance.c‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3735,13 +3735,19 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
37353735
if (ce->ce_flags & ZEND_ACC_UNRESOLVED_VARIANCE) {
37363736
resolve_delayed_variance_obligations(ce);
37373737
}
3738+
/* Delayed variance resolution can re-enter linking before the full
3739+
* hierarchy is linked. See ext/opcache/tests/gh20469*.phpt. */
3740+
if (CG(unlinked_uses) && zend_hash_index_exists(CG(unlinked_uses), (zend_long)(uintptr_t) ce)) {
3741+
ce->ce_flags &= ~ZEND_ACC_CACHEABLE;
3742+
}
37383743
if (ce->ce_flags & ZEND_ACC_CACHEABLE) {
37393744
ce->ce_flags &= ~ZEND_ACC_CACHEABLE;
37403745
} else {
37413746
CG(current_linking_class) = NULL;
37423747
}
37433748
}
37443749

3750+
bool was_cacheable = is_cacheable;
37453751
if (!CG(current_linking_class)) {
37463752
is_cacheable = 0;
37473753
}
@@ -3762,6 +3768,13 @@ ZEND_API zend_class_entry *zend_do_link_class(zend_class_entry *ce, zend_string
37623768
zend_hash_destroy(ht);
37633769
FREE_HASHTABLE(ht);
37643770
}
3771+
} else if (was_cacheable && ce->inheritance_cache) {
3772+
/* Cacheability can be disabled after dependency tracking prepared
3773+
* an inheritance-cache dependency table. Discard it here. */
3774+
HashTable *ht = (HashTable*)ce->inheritance_cache;
3775+
ce->inheritance_cache = NULL;
3776+
zend_hash_destroy(ht);
3777+
FREE_HASHTABLE(ht);
37653778
}
37663779

37673780
if (!orig_record_errors) {

‎ext/opcache/tests/gh20469.phpt‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
--TEST--
2+
GH-20469: Inheritance cache with reentrant autoloading must not crash
3+
--EXTENSIONS--
4+
opcache
5+
--CONFLICTS--
6+
server
7+
--FILE--
8+
<?php
9+
$dir = __DIR__ . '/gh20469';
10+
@mkdir($dir . '/classes', 0777, true);
11+
12+
file_put_contents($dir . '/autoload.php', <<<'PHP'
13+
<?php
14+
spl_autoload_register(function ($class) {
15+
$prefix = 'APP\\';
16+
if (strncmp($class, $prefix, strlen($prefix)) === 0) {
17+
require __DIR__ . '/classes/' . substr($class, strlen($prefix)) . '.php';
18+
}
19+
});
20+
PHP);
21+
22+
/* The dependency cycle is:
23+
* ChildOfParentBeingLinked -> ParentBeingLinked -> CovariantReturnWithTrait
24+
* -> RequiresRootReturnTrait -> ChildOfParentBeingLinked.
25+
*/
26+
file_put_contents($dir . '/test1.php', <<<'PHP'
27+
<?php
28+
require __DIR__ . '/autoload.php';
29+
echo \APP\ChildOfParentBeingLinked::SOME_CONSTANT;
30+
PHP);
31+
32+
file_put_contents($dir . '/test2.php', <<<'PHP'
33+
<?php
34+
require __DIR__ . '/autoload.php';
35+
echo \APP\ParentBeingLinked::SOME_CONSTANT;
36+
PHP);
37+
38+
file_put_contents($dir . '/classes/RootForTraitReturn.php', <<<'PHP'
39+
<?php
40+
namespace APP;
41+
42+
class RootForTraitReturn
43+
{
44+
function createResult(): BaseCovariantReturn
45+
{
46+
}
47+
}
48+
PHP);
49+
50+
file_put_contents($dir . '/classes/ParentBeingLinked.php', <<<'PHP'
51+
<?php
52+
namespace APP;
53+
54+
class ParentBeingLinked extends RootForTraitReturn
55+
{
56+
public const SOME_CONSTANT = 3;
57+
58+
function createResult(): CovariantReturnWithTrait
59+
{
60+
}
61+
}
62+
PHP);
63+
64+
file_put_contents($dir . '/classes/ChildOfParentBeingLinked.php', <<<'PHP'
65+
<?php
66+
namespace APP;
67+
68+
class ChildOfParentBeingLinked extends ParentBeingLinked
69+
{
70+
}
71+
PHP);
72+
73+
file_put_contents($dir . '/classes/BaseCovariantReturn.php', <<<'PHP'
74+
<?php
75+
namespace APP;
76+
77+
abstract class BaseCovariantReturn
78+
{
79+
}
80+
PHP);
81+
82+
file_put_contents($dir . '/classes/RequiresRootReturnTrait.php', <<<'PHP'
83+
<?php
84+
namespace APP;
85+
86+
trait RequiresRootReturnTrait
87+
{
88+
abstract function build(): RootForTraitReturn;
89+
}
90+
PHP);
91+
92+
file_put_contents($dir . '/classes/CovariantReturnWithTrait.php', <<<'PHP'
93+
<?php
94+
namespace APP;
95+
96+
class CovariantReturnWithTrait extends BaseCovariantReturn
97+
{
98+
use RequiresRootReturnTrait;
99+
100+
function build(): ChildOfParentBeingLinked
101+
{
102+
}
103+
}
104+
PHP);
105+
106+
include 'php_cli_server.inc';
107+
$ini = trim((string) getenv('TEST_PHP_EXTRA_ARGS'));
108+
$ini .= ($ini !== '' ? ' ' : '') . '-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.file_update_protection=0';
109+
php_cli_server_start($ini);
110+
111+
echo file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469/test1.php'), "\n";
112+
echo file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469/test2.php'), "\n";
113+
?>
114+
--CLEAN--
115+
<?php
116+
$dir = __DIR__ . '/gh20469';
117+
if (is_dir($dir)) {
118+
$iterator = new RecursiveIteratorIterator(
119+
new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS),
120+
RecursiveIteratorIterator::CHILD_FIRST
121+
);
122+
foreach ($iterator as $file) {
123+
if ($file->isDir()) {
124+
rmdir($file->getPathname());
125+
} else {
126+
unlink($file->getPathname());
127+
}
128+
}
129+
rmdir($dir);
130+
}
131+
?>
132+
--EXPECT--
133+
3
134+
3

‎ext/opcache/tests/gh20469_child_variance_resolves_parent.phpt‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
--TEST--
2+
GH-20469: Child delayed variance can resolve parent before direct delayed resolution
3+
--DESCRIPTION--
4+
This variant ensures the cacheability check after load_delayed_classes() is
5+
needed. Loading the delayed child resolves the parent class's variance
6+
obligations reentrantly, so the parent no longer has ZEND_ACC_UNRESOLVED_VARIANCE
7+
when control returns from load_delayed_classes(). The parent was still used while
8+
nearly linked, and must not be inserted into the inheritance cache.
9+
--EXTENSIONS--
10+
opcache
11+
--CONFLICTS--
12+
server
13+
--FILE--
14+
<?php
15+
$dir = __DIR__ . '/gh20469_child_variance_resolves_parent';
16+
@mkdir($dir . '/classes', 0777, true);
17+
18+
file_put_contents($dir . '/autoload.php', <<<'PHP'
19+
<?php
20+
spl_autoload_register(function ($class) {
21+
$prefix = 'APP\\';
22+
if (strncmp($class, $prefix, strlen($prefix)) === 0) {
23+
require __DIR__ . '/classes/' . substr($class, strlen($prefix)) . '.php';
24+
}
25+
});
26+
PHP);
27+
28+
/* The dependency cycle is:
29+
* ParentBeingLinked -> CovariantReturnWithTrait -> RequiresRootReturnTrait
30+
* -> ChildOfParentBeingLinked -> ParentBeingLinked.
31+
*
32+
* ChildOfParentBeingLinked also has delayed variance, so resolving the child's
33+
* dependency on ParentBeingLinked can resolve ParentBeingLinked before it
34+
* reaches its direct resolve_delayed_variance_obligations() call.
35+
*/
36+
file_put_contents($dir . '/test1.php', <<<'PHP'
37+
<?php
38+
require __DIR__ . '/autoload.php';
39+
// Prime the inheritance cache with the full dependency cycle.
40+
echo \APP\ChildOfParentBeingLinked::SOME_CONSTANT;
41+
PHP);
42+
43+
file_put_contents($dir . '/test2.php', <<<'PHP'
44+
<?php
45+
require __DIR__ . '/autoload.php';
46+
// Link ParentBeingLinked first. During load_delayed_classes(), loading the
47+
// delayed child resolves ParentBeingLinked's variance obligations reentrantly.
48+
echo \APP\ParentBeingLinked::SOME_CONSTANT;
49+
$i = new \APP\ChildOfParentBeingLinked();
50+
var_dump($i->test());
51+
PHP);
52+
53+
file_put_contents($dir . '/test3.php', <<<'PHP'
54+
<?php
55+
require __DIR__ . '/autoload.php';
56+
// Replay the cache state created by test2. If ParentBeingLinked was cached even
57+
// though it was used while nearly linked, this request fails before test() runs.
58+
echo \APP\ParentBeingLinked::SOME_CONSTANT;
59+
$i = new \APP\ChildOfParentBeingLinked();
60+
var_dump($i->test());
61+
PHP);
62+
63+
file_put_contents($dir . '/classes/RootForTraitReturn.php', <<<'PHP'
64+
<?php
65+
namespace APP;
66+
67+
class RootForTraitReturn
68+
{
69+
function createResult(): BaseCovariantReturn
70+
{
71+
}
72+
73+
function test() {}
74+
}
75+
PHP);
76+
77+
file_put_contents($dir . '/classes/ParentBeingLinked.php', <<<'PHP'
78+
<?php
79+
namespace APP;
80+
81+
class ParentBeingLinked extends RootForTraitReturn
82+
{
83+
public const SOME_CONSTANT = 3;
84+
85+
// CovariantReturnWithTrait is unavailable when this method is checked,
86+
// putting ParentBeingLinked into delayed variance resolution.
87+
function createResult(): CovariantReturnWithTrait
88+
{
89+
}
90+
}
91+
PHP);
92+
93+
file_put_contents($dir . '/classes/ChildOfParentBeingLinked.php', <<<'PHP'
94+
<?php
95+
namespace APP;
96+
97+
class ChildOfParentBeingLinked extends ParentBeingLinked
98+
{
99+
// MoreSpecificReturn is also unavailable when the child is linked. Resolving
100+
// this child's delayed variance obligation recursively resolves the parent.
101+
function createResult(): MoreSpecificReturn
102+
{
103+
}
104+
}
105+
PHP);
106+
107+
file_put_contents($dir . '/classes/BaseCovariantReturn.php', <<<'PHP'
108+
<?php
109+
namespace APP;
110+
111+
abstract class BaseCovariantReturn
112+
{
113+
}
114+
PHP);
115+
116+
file_put_contents($dir . '/classes/RequiresRootReturnTrait.php', <<<'PHP'
117+
<?php
118+
namespace APP;
119+
120+
trait RequiresRootReturnTrait
121+
{
122+
abstract function build(): RootForTraitReturn;
123+
}
124+
PHP);
125+
126+
file_put_contents($dir . '/classes/CovariantReturnWithTrait.php', <<<'PHP'
127+
<?php
128+
namespace APP;
129+
130+
class CovariantReturnWithTrait extends BaseCovariantReturn
131+
{
132+
use RequiresRootReturnTrait;
133+
134+
// This pulls ChildOfParentBeingLinked into the delayed autoload queue while
135+
// ParentBeingLinked is nearly linked.
136+
function build(): ChildOfParentBeingLinked
137+
{
138+
}
139+
}
140+
PHP);
141+
142+
file_put_contents($dir . '/classes/MoreSpecificReturn.php', <<<'PHP'
143+
<?php
144+
namespace APP;
145+
146+
class MoreSpecificReturn extends CovariantReturnWithTrait
147+
{
148+
}
149+
PHP);
150+
151+
include 'php_cli_server.inc';
152+
$ini = trim((string) getenv('TEST_PHP_EXTRA_ARGS'));
153+
$ini .= ($ini !== '' ? ' ' : '') . '-d opcache.enable=1 -d opcache.enable_cli=1 -d opcache.file_update_protection=0';
154+
php_cli_server_start($ini);
155+
156+
echo rtrim(file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469_child_variance_resolves_parent/test1.php'), "\n"), "\n";
157+
echo rtrim(file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469_child_variance_resolves_parent/test2.php'), "\n"), "\n";
158+
echo rtrim(file_get_contents('http://' . PHP_CLI_SERVER_ADDRESS . '/gh20469_child_variance_resolves_parent/test3.php'), "\n"), "\n";
159+
?>
160+
--CLEAN--
161+
<?php
162+
$dir = __DIR__ . '/gh20469_child_variance_resolves_parent';
163+
if (is_dir($dir)) {
164+
$iterator = new RecursiveIteratorIterator(
165+
new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS),
166+
RecursiveIteratorIterator::CHILD_FIRST
167+
);
168+
foreach ($iterator as $file) {
169+
if ($file->isDir()) {
170+
rmdir($file->getPathname());
171+
} else {
172+
unlink($file->getPathname());
173+
}
174+
}
175+
rmdir($dir);
176+
}
177+
?>
178+
--EXPECT--
179+
3
180+
3NULL
181+
3NULL

‎ext/opcache/tests/gh20469_inheritance_cache_cleanup.phpt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
GH-20469: Skipped inheritance cache cleanup must ignore non-cacheable classes
3+
--DESCRIPTION--
4+
Autoloading the parent makes the child use the runtime class-linking path, but
5+
the child does not enter inheritance-cache construction. Under ASAN, the
6+
uninitialized inheritance_cache field is filled with non-zero bytes. Skipped
7+
cache insertion must not treat that value as a temporary dependency table.
8+
--EXTENSIONS--
9+
opcache
10+
--FILE--
11+
<?php
12+
spl_autoload_register(function ($class) {
13+
if ($class === 'ParentForSkippedInheritanceCacheCleanup') {
14+
eval('class ParentForSkippedInheritanceCacheCleanup {}');
15+
}
16+
});
17+
18+
eval('class ChildForSkippedInheritanceCacheCleanup extends ParentForSkippedInheritanceCacheCleanup {}');
19+
echo "ok\n";
20+
?>
21+
--EXPECT--
22+
ok

0 commit comments

Comments
 (0)