Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transfer closed for httpd parent and subrequest double bailout #17509

Open
bukka opened this issue Jan 18, 2025 · 1 comment · May be fixed by #17653
Open

Transfer closed for httpd parent and subrequest double bailout #17509

bukka opened this issue Jan 18, 2025 · 1 comment · May be fixed by #17653

Comments

@bukka
Copy link
Member

bukka commented Jan 18, 2025

Description

This can be recreated by setting memory_limit to 128MB and hitting virtual.php script that calls subrequest.php script

The following code:

<?php
// subrequest.php

$mep = str_repeat('test', 256 * 1000000);

echo count($mem);
<?php
// virtual.php

virtual('/subrequest.php');

echo "test";

$mep = str_repeat('test', 256 * 1000000);

echo "virtual!\n";

Resulted in this curl result:

* transfer closed with outstanding read data remaining
* Closing connection 0
curl: (18) transfer closed with outstanding read data remaining

The error logs showed following:

[Sat Jan 18 14:07:36.162766 2025] [php:error] [pid 86048:tid 86048] [client 127.0.0.1:48464] PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted at /home/jakub/prog/php/81/Zend/zend_string.h:163 (tried to allocate 1024000032 bytes) in /home/jakub/prog/php/tests/apache2handler/basic/subrequest.php on line 3
[Sat Jan 18 14:07:36.162842 2025] [php:error] [pid 86048:tid 86048] [client 127.0.0.1:48464] PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted at /home/jakub/prog/php/81/Zend/zend_string.h:163 (tried to allocate 1024000032 bytes) in /home/jakub/prog/php/tests/apache2handler/basic/virtual.php on line 7
/home/jakub/prog/php/81/Zend/zend_alloc.c(393) : Bailed out without a bailout address!

But I expected normal output with 2 errors for allowed memory size

I tested this with 8.1 but it will be the same for supported version I'm sure - will double check later.

PHP Version

PHP 8.1+

Operating System

Ubuntu 20.04

@bukka
Copy link
Member Author

bukka commented Feb 8, 2025

The case above is quite simple one and easily fixable. Unfortunately there are more complex scenarios that make things much worse. Specifically problem reported in https://bugs.php.net/bug.php?id=80558 . Just to simplify it, has a library file:

<?php
// library.php
function foo() {}

Then there is error.php, which is set in ErrorDocument (e.g. ErrorDocument 500 /error.php):

<?php
// error.php
error_reporting(E_ALL);
require_once 'library.php';
 
echo 'I will deal with a 500 error and show a pretty error message';

And finally test.php

<?php
// test.php

error_reporting(E_ALL);
require_once 'library.php';
 
header('Foo : Bar');
flush(); // Only change
echo 'I fell to pieces in a horrendous and nonsensical way';

What happens here that the invalid header is flushed which gets to ap_rflush and due to incorrect header format, the error page is called internally (subrequest) which uses the same globals and everything. That means that it shares already defined foo function and bails out. This will do request shotdown and some other clean up and then it gets back to flush which does another bailout. Without the bailout fix, it will fail because the location is lost. With the fix, it will just segfault because some things got freed.

But even if the error.php runs fine (e.g. commenting out the library require), then it just result in bunch of memleaks and some incorrect handling as well.

It's all just a proper mess of re-using and cleaning the same state. Ideally we should somehow store all persistent globals in the subrequest and then restore them at the end. Seems like quite a big job though. Alternatively we could try to propagate termination to parent so it doesn't clear what has been cleared. Not a small job either but probably more likely for a bug fix.

bukka added a commit to bukka/php-src that referenced this issue Feb 23, 2025
bukka added a commit to bukka/php-src that referenced this issue Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant