PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors #11078
Comments
|
Might be a duplicate of #10670 |
This isn't trying to use custom allocators. I'm unable to trigger a crash, ASAN is also not showing anything. What CURL version are you using? I'm trying this on Linux. |
|
Latest Curl 8.0.1 is used. So far I've triggered the crash on both MacOS and Linux (Docker). The crash is timing dependant so the rate of filling the buffer may need to change depending on your network, it does appear that Curl needs to be past a certain point in the transfer stage for the issue to occur. I'm away from my main computer for a week, I'll come back the. with a code dump from Linux. Is there any other flags I should use for building or running php to help? |
|
That would be great, thank you!
You could try the |
|
Additional address sanitiser output on MacOS Double free output pointer being freed was not allocated output |
|
Alpine Linux produces a different error: From Debian leaks are detected |
|
Depending on where the out of memory error occurs it appears a different order of shutdown. With STREAM_DEBUG enabled. No crash occurs when the Curl handle is closed first (DTOR CALLED before stream_free): Crash occurs if the input file stream is closed before the curl connection is closed (DTOR CALLED after stream_free) |
|
@iluuu1994 please let me know if you need anything additional. I haven't been able to crash reliably on Linux after compiling with debug enabled (Alpine Linux SIGSEGV, Debian I get leaks but no crash). MacOS version crashes reliably every 2-3 invocations of the script with SIGABRT. |
|
I haven't managed to get any further in working out what is going on. I did note that the release of the stream also generates very different debug logs between the shutdown and normally when calling fclose() shutdown fclose |
|
Hi @lucasnetau! Sorry, I've been a little busy the past few days. I'll look into this again soon. |
|
I finally had a closer look. Unfortunately, I still cannot reproduce this issue locally. But like you say, this issue seems to be cause by
I think this can be avoided by calling Edit: On second thought, I'm not sure this changes anything, as the diff --git a/ext/curl/interface.c b/ext/curl/interface.c
index 2dd4a7bf65..7d3910625d 100644
--- a/ext/curl/interface.c
+++ b/ext/curl/interface.c
@@ -408,7 +408,7 @@ PHP_MINIT_FUNCTION(curl)
memcpy(&curl_object_handlers, &std_object_handlers, sizeof(zend_object_handlers));
curl_object_handlers.offset = XtOffsetOf(php_curl, std);
- curl_object_handlers.free_obj = curl_free_obj;
+ curl_object_handlers.dtor_obj = curl_free_obj;
curl_object_handlers.get_gc = curl_get_gc;
curl_object_handlers.get_constructor = curl_get_constructor;
curl_object_handlers.clone_obj = curl_clone_obj;The memory leaks should be addressed by GH-11231. It switches CURL to use |
|
@iluuu1994 good news is the patch above does appear to solve the double free issue, now I get a normal fatal error then exit. I have only been able to replicate this on MacOS. I originally thought I crashed Linux but I haven't been able to replicate the SIGABRT (I was able to get a SIGSEV in Alpine linux) One thing to note is that the piece of memory that cURL is freeing is not the *FILE but the upload buffer which takes data from the file. I had originally posted this to curl/curl#10964, somehow the two are linked but the cURL maintainers don't see how. The bad new is the emalloc patches didn't fix the not allocated error, however after testing I saw you closed that PR: |
curl_easy_cleanup shouldn't be doing that because in curl_multi_free_obj() calls _php_curl_verify_handlers on each cURL handle before freeing which takes care of un-assigning the FILE* that has gone away And from what traces show it is the upload buffer being free'd that causes the double free. |
|
@lucasnetau Thanks for the hint! (I don't know the curl extension well) Now I'm no longer sure how this issue occurs at all... If Just to be sure, you don't have any other extensions enabled that might interact with the issue? Edit: Oh, I missed the last part of your message (it is the upload buffer being free'd that causes the double free). |
|
Nothing out of the ordinary in terms of extensions. PHP is installed via homebrew on MacOS. Yes, the crash line in libcurl is In terms of replicating, it does appear to be timing dependant on how far curl gets with the transfer before the OOM. You can try adjusting the 500 value to something smaller (say 100) or larger depending on your internet speed fwrite($writeStream, str_repeat('x', 500)); Since I can replicate, is there any steps you want me to do to capture any more information? |
|
@lucasnetau I whipped out my dusty old MacBook, I can reproduce the problem there. I'll have a look again soon. |
|
Hi @iluuu1994 , were you ever about to find out anything more with this issue? |
|
Hi @lucasnetau. No, I don't think I ever had another look. I'm not very familiar with curl or the extension. I would be grateful if somebody else could have a look, maybe @adoy? |
|
@SakiTakamachi could you possibly have a look at this as you have a macOS machine? |
|
This reminds me of a mysqlnd bug with streams from some time ago, where the resource was freed before object destruction as well. If this were reproducible on Linux I could give it a debug session, but it seems it's only reproducible on macOS seemingly? Do we know why? |
|
At this time I don't know why this only happens on MacOS. I will try to see if this can be reproduced in an Intel Linux environment. |
|
I haven't tried but could it be a difference between Clang on MacOS and gcc on Linux? |
|
Tried it on my Intel MacBook. It is reproduced on Host OS (Mac OS). However, it does not reproduce on docker ubuntu on Intel MacBook. I've tried it with both gcc and clang (I also built curl from source and tried both patterns) but it doesn't reproduce. |
|
I forgot to write one thing. On my Intel MacBook I always end up with not allocated error. No double free errors occur. edit: At Intel, I was building the master branch. M2 is 8.2. I will align the branches and test them tomorrow. |
|
No need to keep looking for a Linux environment, I got my hands on an old mac system. From the Darwin version I see you're running Sonoma, so I'll give it a go tomorrow. |
|
Thx! I tested the master branch on Arm and got a double free error. |
|
I reproduced this on macOS Ventura. And damn, this issue gave me a headache. The analysis posted in this thread so far isn't right. Curl doesn't contain the code to fclose the For the description below, I will use the FreeBSD libc source code because that's what macOS is based on and I couldn't find the sources of Apple's libc, but low and behold based on what I saw when stepping through Here's what actually happens:
This also explains why we can't reproduce it on Linux: this bizarre buffer swapperoo only happens on macOS and BSD, not on Linux. You can easily prove this by testing this: https://gist.github.com/nielsdos/0d4f15f57ad984944249621e2670a50c So why does it sometimes cause a use-after-free instead of double free? The reason you can change which kind of heap corruption triggers is because you're actually influencing the heap layout by setting an extra option, causing ASAN to draw different conclusions because the shadow bytes are slightly differently layed out. As for the patch, Ilija's patch is not right: we should leave the object in a consistent state (e.g. the As an aside, I see there was a PR at one point to make Curl memory allocations use the request allocator, but it was closed because it isn't thread safe. I'd like to add that while you could enable it in ZTS, but nothing prevents third party extensions or libraries underlying to those extensions from using Curl too and storing that data for longer than a request. That would also cause problems. (That's the reason I can't easily switch libxml to the request allocator.) Giving Curl a unique Another possible temporary solution is using |
|
A possible proper fix at the stream layer is this I think: https://gist.github.com/nielsdos/b47a42bb29f2ababdba35b37eaa3d468 @SakiTakamachi @lucasnetau Can you please test the patch I linked? It should apply cleanly on master. I was only able to test on x86-64 macOS Ventura. |
|
@nielsdos |
|
@nielsdos ,thank you very much for you investigation. I will test out the patch. |
|
@nielsdos I can confirm that this patch fixes the issue for me on both Intel and ARM tested against 8.3 src |
… allocated and malloc: double free for ptr errors Although the issue was demonstrated using Curl, the issue is purely in the streams layer of PHP. Full analysis is written in phpGH-11078 [1], but here is the brief version: Here's what actually happens: 1) We're creating a FILE handle from a stream using the casting mechanism. This will create a cookie-based FILE handle using funopen. 2) We're reading stream data using fread from the userspace stream. This will temporarily set a buffer into a field _bf.base [2]. This buffer is now equal to the upload buffer that Curl allocated and note that that buffer is owned by Curl. 3) The fatal error occurs and we bail out from the fread function, notice how the reset code is never executed and so the buffer will still point to Curl's upload buffer instead of FILE's own buffer [3]. 4) The resources are destroyed, this includes our opened stream and because the FILE handle is cached, it gets destroyed as well. In fact, the stream code calls through fclose on purpose in this case. 5) The fclose code frees the _bs.base buffer [4]. However, this is not the buffer that FILE owns but the one that Curl owns because it isn't reset properly due to the bailout! 6) The objects are getting destroyed, and so the curl free logic is invoked. When Curl tries to gracefully clean up, it tries to free the buffer. But that buffer is actually already freed mistakingly by the C library! This also explains why we can't reproduce it on Linux: this bizarre buffer swapping only happens on macOS and BSD, not on Linux. To solve this, we switch to an unbuffered mode for cookie-based FILEs. This avoids any stateful problems related to buffers especially when the bailout mechanism triggers. As streams have their own buffering mechanism, I don't expect this to impact performance. [1] php#11078 (comment) [2] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103 [3] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117 [4] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67
|
Thanks, I've managed to create an isolated test and opened a PR, let's hope CI agrees too. |
… allocated and malloc: double free for ptr errors Although the issue was demonstrated using Curl, the issue is purely in the streams layer of PHP. Full analysis is written in phpGH-11078 [1], but here is the brief version: Here's what actually happens: 1) We're creating a FILE handle from a stream using the casting mechanism. This will create a cookie-based FILE handle using funopen. 2) We're reading stream data using fread from the userspace stream. This will temporarily set a buffer into a field _bf.base [2]. This buffer is now equal to the upload buffer that Curl allocated and note that that buffer is owned by Curl. 3) The fatal error occurs and we bail out from the fread function, notice how the reset code is never executed and so the buffer will still point to Curl's upload buffer instead of FILE's own buffer [3]. 4) The resources are destroyed, this includes our opened stream and because the FILE handle is cached, it gets destroyed as well. In fact, the stream code calls through fclose on purpose in this case. 5) The fclose code frees the _bs.base buffer [4]. However, this is not the buffer that FILE owns but the one that Curl owns because it isn't reset properly due to the bailout! 6) The objects are getting destroyed, and so the curl free logic is invoked. When Curl tries to gracefully clean up, it tries to free the buffer. But that buffer is actually already freed mistakingly by the C library! This also explains why we can't reproduce it on Linux: this bizarre buffer swapping only happens on macOS and BSD, not on Linux. To solve this, we switch to an unbuffered mode for cookie-based FILEs. This avoids any stateful problems related to buffers especially when the bailout mechanism triggers. As streams have their own buffering mechanism, I don't expect this to impact performance. [1] php#11078 (comment) [2] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103 [3] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117 [4] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67
… allocated and malloc: double free for ptr errors Although the issue was demonstrated using Curl, the issue is purely in the streams layer of PHP. Full analysis is written in phpGH-11078 [1], but here is the brief version: Here's what actually happens: 1) We're creating a FILE handle from a stream using the casting mechanism. This will create a cookie-based FILE handle using funopen. 2) We're reading stream data using fread from the userspace stream. This will temporarily set a buffer into a field _bf.base [2]. This buffer is now equal to the upload buffer that Curl allocated and note that that buffer is owned by Curl. 3) The fatal error occurs and we bail out from the fread function, notice how the reset code is never executed and so the buffer will still point to Curl's upload buffer instead of FILE's own buffer [3]. 4) The resources are destroyed, this includes our opened stream and because the FILE handle is cached, it gets destroyed as well. In fact, the stream code calls through fclose on purpose in this case. 5) The fclose code frees the _bs.base buffer [4]. However, this is not the buffer that FILE owns but the one that Curl owns because it isn't reset properly due to the bailout! 6) The objects are getting destroyed, and so the curl free logic is invoked. When Curl tries to gracefully clean up, it tries to free the buffer. But that buffer is actually already freed mistakingly by the C library! This also explains why we can't reproduce it on Linux: this bizarre buffer swapping only happens on macOS and BSD, not on Linux. To solve this, we switch to an unbuffered mode for cookie-based FILEs. This avoids any stateful problems related to buffers especially when the bailout mechanism triggers. As streams have their own buffering mechanism, I don't expect this to impact performance. [1] php#11078 (comment) [2] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103 [3] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117 [4] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67
… allocated and malloc: double free for ptr errors Although the issue was demonstrated using Curl, the issue is purely in the streams layer of PHP. Full analysis is written in phpGH-11078 [1], but here is the brief version: Here's what actually happens: 1) We're creating a FILE handle from a stream using the casting mechanism. This will create a cookie-based FILE handle using funopen. 2) We're reading stream data using fread from the userspace stream. This will temporarily set a buffer into a field _bf.base [2]. This buffer is now equal to the upload buffer that Curl allocated and note that that buffer is owned by Curl. 3) The fatal error occurs and we bail out from the fread function, notice how the reset code is never executed and so the buffer will still point to Curl's upload buffer instead of FILE's own buffer [3]. 4) The resources are destroyed, this includes our opened stream and because the FILE handle is cached, it gets destroyed as well. In fact, the stream code calls through fclose on purpose in this case. 5) The fclose code frees the _bs.base buffer [4]. However, this is not the buffer that FILE owns but the one that Curl owns because it isn't reset properly due to the bailout! 6) The objects are getting destroyed, and so the curl free logic is invoked. When Curl tries to gracefully clean up, it tries to free the buffer. But that buffer is actually already freed mistakingly by the C library! This also explains why we can't reproduce it on Linux: this bizarre buffer swapping only happens on macOS and BSD, not on Linux. To solve this, we switch to an unbuffered mode for cookie-based FILEs. This avoids any stateful problems related to buffers especially when the bailout mechanism triggers. As streams have their own buffering mechanism, I don't expect this to impact performance. [1] php#11078 (comment) [2] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L102-L103 [3] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fread.c#L117 [4] https://github.com/apple-open-source-mirror/Libc/blob/5e566be7a7047360adfb35ffc44c6a019a854bea/stdio/FreeBSD/fclose.c#L66-L67
|
In doing some benchmark testing with curl uploads via the stream I also discovered that this patch also fixed an issue I was seeing where cURL stopped uploading after stream_read() returned In the testing I was able to do, cURL would resume however it never called the stream_read() leading to no data being transferred. Performance wise, I don't see an variance between old and patched version POSTing a large file to a localhost server, both sat at around 1350 MB/s throughput |
* PHP-8.2: Fix GH-11078: PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors
* PHP-8.3: Fix GH-11078: PHP Fatal error triggers pointer being freed was not allocated and malloc: double free for ptr errors
|
@lucasnetau Thanks for the results, fix will be in the next release. |
Description
The following code:
https://gist.github.com/lucasnetau/244ba31f321307e06177f48582273e86
Resulted in this output:
To trigger the issue a PHP Fatal error (out of memory) needs to occur in a write to a custom streamWrapper, the test case forces this quickly (pointer issue occurs on each run, double free occurs on most runs but not all). Notably when CURLOPT_HTTPHEADER is set the double free occurs in libcurl, when CURLOPT_HTTPHEADER is not set then the point being freed error occurs.
Double Free backtrace:
Pointer being freed was not allocated:
Double free was reported to curl via curl/curl#10964, however since a subtle change in settings (CURLOPT_HTTPHEADER) triggers a crash in PHP I am posting both here.
With ext-curl compiled with PHP_CURL_DEBUG=0 I only see the DTOR being called once (DTOR CALLED, ch = *)
The comments in interface.c before where the crash occurs for Curl are interesting
https://github.com/php/php-src/blob/master/ext/curl/interface.c#L2829-2838
PHP Version
PHP 8.2.5
Operating System
MacOS / Linux
The text was updated successfully, but these errors were encountered: