Edit report at http://bugs.php.net/bug.php?id=53567&edit=1
ID: 53567
User updated by: algernon at balabit dot hu
Reported by: algernon at balabit dot hu
Summary: PDO's mishandling of peristence, leading to a
segfault
Status: Open
Type: Bug
Package: PDO related
Operating System: Linux
PHP Version: Irrelevant
Block user comment: N
Private report: N
New Comment:
You're right - that's indeed a scenario I didn't consider.
I don't have the source code before me at the moment, but an option
would be to
free and clear ->query_stmt and ->query_stmt_zval before putting the DBH
object
into the cache. However, I have a feeling that would have a few unwanted
side
effects too.
Thinking further... would it be possible to decouple ->query_stmt and -
>query_stmt zval from the DBH object, and store them somewhere else? If
they
never reach the cache, then there is no problem.
Actually, if we just copy DBH before putting it into the cache, with
these two
fields blanked out, that might be enough, perhaps. Though, that might
lead to
another kind of memory leak - without being able to have a quick look,
I'm not
quite sure.
Previous Comments:
------------------------------------------------------------------------
[2010-12-18 17:26:36] cataphr...@php.net
I'm unsure about the attached patch.
If there's the exception and PHP bails out, zeroing the zval field is of
course correct. But what if the PDO error doesn't generate exceptions
(no PDO::ERRMODE_EXCEPTION) and you create another object? You'll be
zeroing the field from cache and leaking the object associated with the
zval because there's no call to zval_dtor, right?
Leaked objects are reclaimed on request shutdown, but another solution
would be preferable.
------------------------------------------------------------------------
[2010-12-17 22:06:46] algernon at balabit dot hu
Description:
------------
We observed a segmentation fault in PHP, which we tracked back to PDO,
and in
particular, the way it handles persistence. I will go into detail just a
tiny
bit later - we'll need a suitable environment to reproduce the bug
first.
For the record, we used the stock PHP5 packages that come with Ubuntu
Lucid, but
a quick look at SVN trunk as of yesterday suggests that the issue is
present
even in trunk. We did not test that, though.
We were using lighttpd with PHP over FastCGI: example configs can be
found all
around, there's nothing irregular in our setup. The key here, is FastCGI
(though, other scenarios where the zend_hash_* stuff is used might also
be
affected).
When PDO notices persistence is enabled, it will put the whole dbh
structure
into the persistent cache, near the end of the constructor. The next
time the
constructor is called, it will grab the dbh structure from the cache
as-is.
This would work, except that the dbh structure contains ->query_stmt and
-
>query_stmt_zval, which can potentially point to free'd memory: the last
statement is stored in these variables, but in case we issue an invalid
SQL
query, and get an uncaught exception back, PHP will bail out, without
calling
the PDO destructor, thus, ->query_stmt and ->query_stmt_zval will point
to
garbage.
This again wouldn't be a problem, if dbh wouldn't get reused. But
remember: it
has been stored in the cache, and the next time the PDO constructor gets
called,
it will grab dbh out of the cache, with two variables pointing to
garbage.
And then all hell breaks loose.
To reproduce the problem, the test script below can be used. For it to
work, one
will need any kind of database (the script uses postgres, but any driver
should
work) - the database can be empty, neither query requires any tables.
The script
will make two queries: a correct one, and a bad one. We need the bad one
to
trigger an exception.
Once the script is in place, I used the three terminals with a wget loop
each
(there might be easier ways to trigger the bug, but this ain't that bad,
either):
On the first terminal:
$ while true ; do wget -O /dev/null 'http://localhost/?sleep=10' ; sleep
2 ;
done
On the second and third:
$ while true ; do wget -O /dev/null 'http://localhost/?foo=bar' ; sleep
2 ; done
Within a couple of seconds, I started to get HTTP 500 errors and core
files.
The fix (see the patch attached), is to zero out ->query_stmt and -
>query_stmt_zval after retrieving a DBH object from the persistent
storage.
(Many thanks to my collegues Attila Magyar <at...@balabit.hu> & Balázs
Scheidler
<ba...@balabit.hu> for doing the bulk of debugging & researching the
issue)
Test script:
---------------
<?php
class A
{
public function __construct() {
$this->pdo = new PDO('pgsql:dbname=sftest;', 'user', 's1krl+',
array(PDO::ATTR_PERSISTENT => true));
$this->pdo->setAttribute(PDO::ATTR_ERRMODE,
PDO::ERRMODE_EXCEPTION);
}
public function __destruct() {
unset($this->pdo);
}
public function trigger() {
$result = $this->pdo->query('SELECT 42 AS fortytwo;');
sleep ($_GET['sleep']);
$result = $this->pdo->query ('SELECT ICanHasFail();');
}
private $pdo;
}
$a = new A();
$a->trigger();
?>
Expected result:
----------------
The expected result would be anything but core files and segfaults.
Actual result:
--------------
Core was generated by `/usr/bin/php-cgi'.
Program terminated with signal 11, Segmentation fault.
#0 0x00000000006bb7a2 in zend_objects_store_del_ref_by_handle_ex
(handle=0,
handlers=0x2a66400) at /build/buildd/php5-5.3.2/Zend/zend_gc.h:190
190 root->prev->next = root->next;
(gdb) bt
#0 0x00000000006bb7a2 in zend_objects_store_del_ref_by_handle_ex
(handle=0,
handlers=0x2a66400) at /build/buildd/php5-5.3.2/Zend/zend_gc.h:190
#1 0x00000000006bb813 in zend_objects_store_del_ref (zobject=0x2a8d1e8)
at
/build/buildd/php5-5.3.2/Zend/zend_objects_API.c:172
#2 0x00007fe3bbc877b3 in pdo_dbh_attribute_set (dbh=0x2a8d140, attr=12,
value=0x2a68c90) at /build/buildd/php5-5.3.2/ext/pdo/pdo_dbh.c:825
#3 0x00007fe3bbc88275 in zim_PDO_dbh_constructor (ht=4,
return_value=0x2a66400,
return_value_ptr=0xd92340, this_ptr=0x7fffae890f68, return_value_used=0)
at
/build/buildd/php5-5.3.2/ext/pdo/pdo_dbh.c:409
#4 0x00000000006e719a in zend_do_fcall_common_helper_SPEC
(execute_data=0x2a96318) at
/build/buildd/php5-5.3.2/Zend/zend_vm_execute.h:313
#5 0x00000000006be480 in execute (op_array=0x2a68f20) at
/build/buildd/php5-
5.3.2/Zend/zend_vm_execute.h:104
#6 0x00000000006961ad in zend_execute_scripts (type=0,
retval=0x7fffae891390,
file_count=3) at /build/buildd/php5-5.3.2/Zend/zend.c:1266
#7 0x0000000000641df8 in php_execute_script (primary_file=0x2) at
/build/buildd/php5-5.3.2/main/main.c:2288
#8 0x0000000000723d44 in main (argc=32767, argv=0x0) at
/build/buildd/php5-
5.3.2/sapi/cgi/cgi_main.c:2110
(gdb) fr 3
#3 0x00007fe3bbc88275 in zim_PDO_dbh_constructor (ht=4,
return_value=0x2a66400,
return_value_ptr=0xd92340, this_ptr=0x7fffae890f68, return_value_used=0)
at
/build/buildd/php5-5.3.2/ext/pdo/pdo_dbh.c:409
409 && HASH_KEY_IS_LONG ==
zend_hash_get_current_key(Z_ARRVAL_P(options), &str_key, &long_key, 0))
{
(gdb) print dbh->query_stmt_zval
$2 = {value = {lval = 4, dval = 1.9762625833649862e-323, str = {val =
0x4
<Address 0x4 out of bounds>, len = -1142328224}, ht = 0x4, obj = {handle
= 4,
handlers = 0x7fe3bbe97460}}, refcount__gc = 2,
type = 5 '\005', is_ref__gc = 1 '\001'}
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/bug.php?id=53567&edit=1
