php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
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: Sun Oct 27 16:01:27 2024 UTC