php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #71569 #70389 fix causes segmentation fault
Submitted: 2016-02-11 11:56 UTC Modified: 2016-02-13 21:58 UTC
From: pavel2000 at ngs dot ru Assigned: nikic (profile)
Status: Closed Package: PDO related
PHP Version: 5.6.18 OS:
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: pavel2000 at ngs dot ru
New email:
PHP Version: OS:

 

 [2016-02-11 11:56 UTC] pavel2000 at ngs dot ru 
Description:
------------
Hi!

We began to get segfaults after updating 5.6.7 up to 5.6.17 on our Debian 7 box.

While searching for the causes of this, we found that resolution of 
 https://bugs.php.net/bug.php?id=70389 (http://git.php.net/?p=php-src.git;a=commit;h=ef1bd8f0e6f88b1d123cea1c0b5079cfde7f90df) introduces regression.

Please look into the backtrace we get:

Program received signal SIGSEGV, Segmentation fault.
_zend_mm_free_int (heap=0xdc7c30, p=0x7ffff7ea4058) at /home/t/php-5.6.17/Zend/zend_alloc.c:2104
2104    /home/t/php-5.6.17/Zend/zend_alloc.c: No such file or directory.
(gdb) bt
#0  _zend_mm_free_int (heap=0xdc7c30, p=0x7ffff7ea4058) at /home/t/php-5.6.17/Zend/zend_alloc.c:2104
#1  0x00007fffe6543c05 in pdo_mysql_handle_factory (dbh=0x25cbd00, driver_options=0x22e5d68)
    at /home/t/php-5.6.17/ext/pdo_mysql/mysql_driver.c:685
#2  0x00007ffff5d2f1ba in zim_PDO_dbh_constructor () at /build/php5-5.6.17+dfsg/ext/pdo/pdo_dbh.c:389
#3  0x00000000006e5cd2 in zend_do_fcall_common_helper_SPEC (execute_data=0x7ffff7fac858) at /home/t/php-5.6.17/Zend/zend_vm_execute.h:558
#4  0x00000000006aa728 in execute_ex (execute_data=0x7ffff7fac858) at /home/t/php-5.6.17/Zend/zend_vm_execute.h:363
#5  0x0000000000642a89 in zend_execute_scripts (type=type@entry=8, retval=retval@entry=0x0, file_count=file_count@entry=3)
    at /home/t/php-5.6.17/Zend/zend.c:1341
#6  0x00000000005df042 in php_execute_script (primary_file=primary_file@entry=0x7fffffffd290) at /home/t/php-5.6.17/main/main.c:2597
#7  0x00000000006e8f79 in do_cli (argc=6, argv=0xdc7890) at /home/t/php-5.6.17/sapi/cli/php_cli.c:994
#8  0x0000000000427a1d in main (argc=6, argv=0xdc7890) at /home/t/php-5.6.17/sapi/cli/php_cli.c:1378


mysql_driver.c has following lines of this process:

687:  default_group= pdo_attr_strval(driver_options, PDO_MYSQL_ATTR_READ_DEFAULT_GROUP, NULL TSRMLS_CC);
...
685:  efree(default_group)   <--- crashed here


Fix #70389 changes pdo_attr_strval() implementation.
After replacing pdo_mysql.so to file from 5.6.7 problem disappears (and #70389 comes back). All indicates that the reason for crashes we got is #70389 regression.

Please note, what we use MYSQL_ATTR_READ_DEFAULT_GROUP flag in our code, it was set it to string value:

            PDO::MYSQL_ATTR_READ_DEFAULT_GROUP => 'client-section'


Sorry, I'm unable to provide more info such as test PHP script or a patch, as I'm not PHP or C qualified developer. But I think my ticket and backtrace can be useful point of start to review #70389 / ef1bd8f0e6f88b1d123cea1c0b5079cfde7f90df again.

Thanks.

Patches

patch (last revision 2016-02-13 12:20 UTC by pavel2000 at ngs dot ru)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-02-11 11:58 UTC] pavel2000 at ngs dot ru 
bt full 2

#0  _zend_mm_free_int (heap=0xdc7c30, p=0x7ffff7ea4058) at /home/t/php-5.6.17/Zend/zend_alloc.c:2104
        mm_block = 0x7ffff7ea4048
        next_block = 0xffffefdd3c68
        size = 140737353284640

#1  0x00007fffe6543c05 in pdo_mysql_handle_factory (dbh=0x25cbd00, driver_options=0x22e5d68)
    at /home/t/php-5.6.17/ext/pdo_mysql/mysql_driver.c:685
        connect_timeout = 30
        default_file = <optimized out>
        ssl_capath = 0x0
        init_cmd = <optimized out>
        default_group = <optimized out>
        ssl_key = 0x0
        ssl_cert = 0x0
        ssl_cipher = 0x0
        compress = 0
        ssl_ca = 0x0
        H = 0x25a6300
        i = <optimized out>
        ret = 0
        host = 0x0
        unix_socket = 0x0
        port = 3306
        dbname = <optimized out>
        vars = {{optname = 0x7fffe6546045 "charset", optval = 0x25c5b70 "UTF8", freeme = 1}, {optname = 0x7fffe654604d "dbname",
            optval = 0x25c5be8 "test2", freeme = 1}, {optname = 0x7fffe6546059 "host", optval = 0x25bc2d0 "127.0.0.1", freeme = 1}, {
            optname = 0x7fffe654605e "port", optval = 0x7fffe6546063 "3306", freeme = 0}, {optname = 0x7fffe6546068 "unix_socket",
            optval = 0x25c2388 "/var/run/mysqld/mysqld.sock", freeme = 1}}
        connect_opts = 196608
 [2016-02-13 12:17 UTC] pavel2000 at ngs dot ru 
Hi!

We continued to discover this issue.

Our crashes caused by attribute MYSQL_ATTR_READ_DEFAULT_GROUP with NULL value and many other conditions.

When Z_TYPE is 0 (IS_NULL) then convert_to_string() returns value obtained from STR_EMPTY_ALLOC(). In turn, STR_EMPTY_ALLOC() returns a value of CG(interned_empty_string). 
I don't think what doing efree() on this value later (like ext/pdo_mysql/mysql_driver.c:685) is a good idea.

Unless ef1bd8f0e6f88b1d123cea1c0b5079cfde7f90df applied, there was no efree() on this value due to estrndup() call, but now it is.

This needs to be fixed, see proposed patch attached.
 [2016-02-13 14:22 UTC] nikic@php.net 
Automatic comment on behalf of nikic
Revision: http://git.php.net/?p=php-src.git;a=commit;h=bc419fee5c9704eb4ce338acacbc2380c6f4427d
Log: FIx bug #71569
 [2016-02-13 14:22 UTC] nikic@php.net
-Status: Open +Status: Closed
 [2016-02-13 14:28 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2016-02-13 14:28 UTC] nikic@php.net 
@pavel: Thanks for the investigation. As you correctly deduced the bug was caused by efreeing an interned string. I've fixed this by using an interned string compatible freeing function instead.
 [2016-02-13 15:23 UTC] pavel2000 at ngs dot ru 
Hi Nikita!

Thats solution is better than my! Thanks for your patch!

Also I want to notice you about wrong description in
ext/pdo_mysql/tests/bug71569.phpt  :

> --- /dev/null
> +++ b/ext/pdo_mysql/tests/bug71569.phpt
> @@ -0,0 +1,23 @@
> +--TEST--
> +Bug #70389 (PDO constructor changes unrelated variables)
> +--SKIPIF--

It was left unchanged from #70389.

Thanks again.
 [2016-02-13 21:58 UTC] nikic@php.net 
Thanks, I've fixed the description in https://github.com/php/php-src/commit/adcdb4f7bab6a0c59fbcca108fac526d8ede70e6.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Nov 23 07:01:29 2024 UTC