Apache crash on Windows when using a self-referencing anonymous function inside a class with an active mysqli connection #10599

DaGaMx · 2023-02-15T12:54:06Z

Description

The following code:

<?php

$x = new X();
$x->y();

class X
{
	public function __construct()
	{
		$this->link = @new mysqli();
		@$this->link->connect('.', 'root', 'password', 'instance', null, '\\\\.\\pipe\\pipeName');
	}

	public function y()
	{
		$parseConnection = null;
		$parseConnection = function() use (&$parseConnection) {
		};
	}
}

Resulted in this output:

The Apache service crashes after accessing this page multiple times (reload the page 2 to 5 times).
In the Windows Event viewer you can find this message (sorry for the German text, but the OS is in German):
--------------------------------------------------------------
Name der fehlerhaften Anwendung: httpd.exe, Version: 21.3.0.49152, Zeitstempel: 0x61646032
Name des fehlerhaften Moduls: php8ts.dll, Version: 8.1.14.0, Zeitstempel: 0x63b57733
Ausnahmecode: 0xc0000005
Fehleroffset: 0x000000000005cc55
ID des fehlerhaften Prozesses: 0xdfc
Startzeit der fehlerhaften Anwendung: 0x01d9413adafb9578
Pfad der fehlerhaften Anwendung: C:\apache\bin\httpd.exe
Pfad des fehlerhaften Moduls: C:\apache\php-8.1.14-Win32-vs16-x64\php8ts.dll
Berichtskennung: b79274d7-45ed-48f8-a592-478bcbe8df41
Vollständiger Name des fehlerhaften Pakets: 
Anwendungs-ID, die relativ zum fehlerhaften Paket ist:

But I expected this output instead:

It should just work, without any process crashes.
In PHP 7.3 this code works fine. Starting with PHP 7.4 I got the crash.

I've tested much to find this bug. It only appears if you have a mysqli connection inside of the class instance.
Also you need to use a anonymous function with a use of the variable by reference where you assign the anonymous function.

As a workaround for me I will get rid of this anonymous function now. But however it should not be possible to crash the executing process like this.

PHP Version

PHP 8.1.14 (the bug exists since 7.4, in 7.3 there is no crash)

Operating System

Windows

The text was updated successfully, but these errors were encountered:

Girgias · 2023-02-20T14:21:19Z

@kamil-tekiela can you maybe have a look at this to see if you can reproduce it? Don't remember the sets to getting ext/mysqli test suit running locally

kamil-tekiela · 2023-02-20T20:03:06Z

I can reproduce it on Windows.

Zend\zend_list.c(42) : Freeing 0x000001da72659ac0 (32 bytes)

The MCVE is:

<?php

$link = new mysqli('.', 'root', '', 'test', null, '\\\\.\\pipe\\pipeName');

This begs the question: if it's so easily reproducible, does this mean nobody uses this feature?

nielsdos · 2024-03-09T23:25:55Z

This begs the question: if it's so easily reproducible, does this mean nobody uses this feature?

Because it only crashes on shutdown, so for most SAPIs that takes a while until it crashes; and the SAPI just start the process automatically on a crash. It's very visible on CGI and CLI though where it will crash after handling the request/file.

What happens is that the persistent network stream resource gets freed, yet stays inside EG(persistent_list). This causes a crash on shutdown when the persistent list is getting cleared, as the engine will try to free the network stream again.
The code at fault is in MYSQLND_METHOD(mysqlnd_vio, close_stream), where it does the mixup between persistent vs non-persistent destruction:

php-src/ext/mysqlnd/mysqlnd_vio.c

Lines 654 to 665 in bb1ef4f

    
           		/* We removed the resource from the stream, so pass FREE_RSRC_DTOR now to force 
        
           		 * destruction to occur during shutdown, because it won't happen through the resource. */ 
        
           		/* TODO: The EG(active) check here is dead -- check IN_SHUTDOWN? */ 
        
           		if (pers && EG(active)) { 
        
           			php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE_PERSISTENT | PHP_STREAM_FREE_RSRC_DTOR); 
        
           		} else { 
        
           			/* 
        
           			  otherwise we will crash because the EG(persistent_list) has been freed already, 
        
           			  before the modules are shut down 
        
           			*/ 
        
           			php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR); 
        
           		}

Notice how it checks for EG(active) to check if we're in shutdown or not. The The EG(active) check here is dead -- check IN_SHUTDOWN? comment is misleading. This code was introduced in c3019a1 to fix crashes when the persistent list gets destroyed before module shutdown is called. This is indeed a potential problem that I fixed permanently on the master branch in 5941cda

So for master, the fix it simple: remove that EG(active) check and remove the misleading comment and it works. I checked this change, and indeed, when trying on Windows with OP's code I no longer get an ASAN error.

On lower branches, it's trickier. Although on the surface level it seems that the same fix also works there (I tried this), it might still run into the issue that the persistent list is destroyed, although I'm not entirely certain how hard it is to trigger that. Then again, if we crash already with the existing code, we might as well change it anyway?

In any case: on master it's a safe fix, on lower branches not necessarily.

I could create a PR for master, and leave this as-is for lower branches; the only thing that remains then is how we're going to create a phpt file for this. This would need a named pipe (or are there other ways to reproduce this too?) and I'm not sure how to set that up into CI...

@kamil-tekiela If you have any thoughts on this, I'd be happy to read them 🙂

kamil-tekiela · 2024-03-21T12:26:11Z

@nielsdos If you can fix it on master and test locally that it resolves the problem then I think that would be sufficient for now. We can keep this issue open and think how to add a proper test and fix it on lower branches.

What happens is that the persistent network stream resource gets freed, yet stays inside EG(persistent_list). This causes a crash on shutdown when the persistent list is getting cleared, as the engine will try to free the network stream again. The code in close_stream gets confused between persistent vs non-persistent allocations when EG(active) is false. This code was introduced in c3019a1 to fix crashes when the persistent list gets destroyed before module shutdown is called. This is indeed a potential problem that was fixed on the master branch in 5941cda. This fixes the crash reason of phpGH-10599.

This fixes the memory leak part of phpGH-10599.

This prevents the code from getting desynced again, which was the reason for the leak of phpGH-10599.

…i connection The code originally posted in phpGH-10599 triggers the bug for non-persistent connections, but changing the host to `p:.` reveals that there is also a crash bug for persistent connections.

What happens is that the persistent network stream resource gets freed, yet stays inside EG(persistent_list). This causes a crash on shutdown when the persistent list is getting cleared, as the engine will try to free the network stream again. The code in close_stream gets confused between persistent vs non-persistent allocations when EG(active) is false. This code was introduced in c3019a1 to fix crashes when the persistent list gets destroyed before module shutdown is called. This is indeed a potential problem that was fixed on the master branch in 5941cda. This fixes the crash reason of phpGH-10599.

This fixes the memory leak part of phpGH-10599.

This prevents the code from getting desynced again, which was the reason for the leak of phpGH-10599.

…i connection The code originally posted in phpGH-10599 triggers the bug for non-persistent connections, but changing the host to `p:.` reveals that there is also a crash bug for persistent connections.

What happens is that the persistent network stream resource gets freed, yet stays inside EG(persistent_list). This causes a crash on shutdown when the persistent list is getting cleared, as the engine will try to free the network stream again. The code in close_stream gets confused between persistent vs non-persistent allocations when EG(active) is false. This code was introduced in c3019a1 to fix crashes when the persistent list gets destroyed before module shutdown is called. This is indeed a potential problem that was fixed on the master branch in 5941cda. This fixes the crash reason of GH-10599.

This fixes the memory leak part of GH-10599.

This prevents the code from getting desynced again, which was the reason for the leak of GH-10599.

…i connection The code originally posted in GH-10599 triggers the bug for non-persistent connections, but changing the host to `p:.` reveals that there is also a crash bug for persistent connections.

This partially backports that PR to stable branches as it has been in master without reported problems so far. It's only a partial backport because the stable branches don't have the ZTS persistent resource fix that would fix shutdown crashes, i.e. the code change in mysqlnd_vio's close_stream is not backported. Closes phpGH-10599.

DaGaMx added Bug Status: Needs Triage labels Feb 15, 2023

kamil-tekiela added Status: Verified and removed Status: Needs Triage labels Feb 20, 2023

nielsdos added the Extension: mysqlnd label Mar 9, 2024

nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 21, 2024

Fix memory leak in mysqlnd_vio::open_pipe

4744b84

This fixes the memory leak part of phpGH-10599.

nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 21, 2024

Factor common code out into mysqlnd_fixup_regular_list()

0f2629b

This prevents the code from getting desynced again, which was the reason for the leak of phpGH-10599.

nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 21, 2024

Fix memory leak in mysqlnd_vio::open_pipe

e98c7ee

This fixes the memory leak part of phpGH-10599.

nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 21, 2024

Factor common code out into mysqlnd_fixup_regular_list()

c04565a

This prevents the code from getting desynced again, which was the reason for the leak of phpGH-10599.

nielsdos self-assigned this Mar 21, 2024

nielsdos mentioned this issue Mar 21, 2024

Fix mysqlnd pipe crash #13782

Closed

nielsdos added a commit that referenced this issue Apr 21, 2024

Fix memory leak in mysqlnd_vio::open_pipe

319fb50

This fixes the memory leak part of GH-10599.

nielsdos added a commit that referenced this issue Apr 21, 2024

Factor common code out into mysqlnd_fixup_regular_list()

a20d9d7

This prevents the code from getting desynced again, which was the reason for the leak of GH-10599.

nielsdos mentioned this issue May 25, 2024

Partially backport GH-13782 to stable branches #14324

Closed

nielsdos closed this as completed in c815cdc May 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apache crash on Windows when using a self-referencing anonymous function inside a class with an active mysqli connection #10599

Apache crash on Windows when using a self-referencing anonymous function inside a class with an active mysqli connection #10599

Apache crash on Windows when using a self-referencing anonymous function inside a class with an active mysqli connection #10599

Apache crash on Windows when using a self-referencing anonymous function inside a class with an active mysqli connection #10599

Comments

Description

PHP Version

Operating System