Edit report at https://bugs.php.net/bug.php?id=71115&edit=1
ID: 71115
Updated by: [email protected]
Reported by: bobby dot mihalca at touchtech dot ro
Summary: data race on auto globals names and internal functions
names refcount
-Status: Feedback
+Status: No Feedback
Type: Bug
Package: Reproducible crash
Operating System: Any
PHP Version: 7.0.0
Assigned To: ab
Private report: N
New Comment:
No feedback was provided. The bug is being suspended because
we assume that you are no longer experiencing the problem.
If this is not the case and you are able to provide the
information that was requested earlier, please do so and
change the status of the bug back to "Re-Opened". Thank you.
Previous Comments:
------------------------------------------------------------------------
[2015-12-18 13:27:45] [email protected]
Related To: Bug #71129
------------------------------------------------------------------------
[2015-12-15 12:04:22] [email protected]
I have forwarded bobby and anatol a poc solution ... one that breaks the rest of PHP, but I think we can do it with small changes contained in zend_string.* without breaking ABI or API.
------------------------------------------------------------------------
[2015-12-15 11:04:08] bobby dot mihalca at touchtech dot ro
@ab
- Multiple threads reading read only global data is not of an issue.
- Same mechanics does exist in PHP5, disregarding zend_string.
Yes but,
php 5 uses const char *, copying a const char * is read only operation
php 7 uses zend_string*, copying a zend_string* is NOT read only as writes refcount.
This is the problem, it copies global zend_string* data concurrently causing data race, causing refcount==0 instead of >=1, causing string free, causing crash.
Global data zend_string* needs to be read only, no refcount without exclusive lock.
Wish it was this clear on initial bug report :)
------------------------------------------------------------------------
[2015-12-15 10:39:26] bobby dot mihalca at touchtech dot ro
@krakjoe
Yes is a hack, it only works because the following stars are aligned :)
* strings are already persistent
* no interned implemented for TS, (hopefully) nothing to mess up
* refcount not used for interned strings
My assumption is that since the names are init during module startup, there should already be code to release them during module shutdown, else we would have a leak now ?
Also i'm assuming no request can be running at module shutdown so nobody other then module holds a pointer.
I should have not brought up the workaround, i was trying to help but only managed to distract and confuse with it.
------------------------------------------------------------------------
[2015-12-15 06:31:25] [email protected]
I think bobby knows it's a hack, he's saying the result is correct, and I agree with him.
A persistent string should not be reference counted, since it can lead to breaking the API's promise and freeing the string early, which is the problem here.
The problem is that the solution used is a hack to get the desired result, but I'm not sure that we can do the same thing internally because I can't see an opportunity to call the final zend_string_free on all strings allocated persistently.
What we may need to do is store persistent strings in some truly global (safe) structure and destroy them at process shutdown. Then change zend_string_copy to omit increment, and zend_string_release to omit decrement, if & IS_STR_PERSISTENT, and change every use of GC_REFCOUNT(str)++|-- to use the API.
I haven't thought about it for very long, but some of that seems to make sense ... I'll keep thinking about it ...
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=71115
--
Edit this bug report at https://bugs.php.net/bug.php?id=71115&edit=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.