Fixed #74021 - fetch_array returned broken data #2357

Closed
wants to merge 1 commit into
from

Conversation

3 participants

@krakjoe krakjoe added the Bugfix label Feb 1, 2017

@nikic

This comment has been minimized.

Show comment
Hide comment
@nikic

nikic Feb 1, 2017

Member

I don't think the linked commit fixes the issue from #2249. It only concerns bit fields.

Member

nikic commented Feb 1, 2017

I don't think the linked commit fixes the issue from #2249. It only concerns bit fields.

@andrewnester

This comment has been minimized.

Show comment
Hide comment
@andrewnester

andrewnester Feb 1, 2017

Contributor

@nikic I've added bug73800.phpt to check that #2249 doesn't occur.

Contributor

andrewnester commented Feb 1, 2017

@nikic I've added bug73800.phpt to check that #2249 doesn't occur.

@nikic nikic self-requested a review Feb 1, 2017

@nikic nikic self-assigned this Feb 12, 2017

@nikic

This comment has been minimized.

Show comment
Hide comment
@nikic

nikic Feb 12, 2017

Member

As suspected, when running the test for bug 73800 you added under valgrind, I get memory errors:

==26369== Invalid read of size 1
==26369==    at 0xABBE49: php_mysqlnd_rowp_read_text_protocol_aux (mysqlnd_wireprotocol.c:1703)
==26369==    by 0xABC57F: php_mysqlnd_rowp_read_text_protocol_zval (mysqlnd_wireprotocol.c:1787)
==26369==    by 0xAF0927: mysqlnd_mysqlnd_result_buffered_zval_fetch_row_pub (mysqlnd_result.c:1096)
==26369==    by 0xAF2146: mysqlnd_mysqlnd_res_fetch_row_pub (mysqlnd_result.c:1275)
==26369==    by 0xAF6BFD: mysqlnd_mysqlnd_res_fetch_into_pub (mysqlnd_result.c:1754)
==26369==    by 0x7C80A6: php_mysqli_fetch_into_hash_aux (mysqli.c:1220)
==26369==    by 0x7C843E: php_mysqli_fetch_into_hash (mysqli.c:1271)
==26369==    by 0x7D836B: zif_mysqli_fetch_array (mysqli_nonapi.c:350)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)
==26369==    by 0xC74825: execute_ex (zend_vm_execute.h:432)
==26369==    by 0xC74AC4: zend_execute (zend_vm_execute.h:474)
==26369==    by 0xC05E7A: zend_execute_scripts (zend.c:1544)
==26369==  Address 0x11d9d8dc is 0 bytes after a block of size 18,000,028 alloc'd
==26369==    at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26369==    by 0xBC29E5: __zend_realloc (zend_alloc.c:2836)
==26369==    by 0xBC1A1F: _erealloc (zend_alloc.c:2443)
==26369==    by 0xAA2A22: _mysqlnd_erealloc (mysqlnd_alloc.c:267)
==26369==    by 0xB10147: mysqlnd_mempool_resize_chunk (mysqlnd_block_alloc.c:94)
==26369==    by 0xAB9B4A: php_mysqlnd_read_row_ex (mysqlnd_wireprotocol.c:1511)
==26369==    by 0xABCCFD: php_mysqlnd_rowp_read (mysqlnd_wireprotocol.c:1826)
==26369==    by 0xAF26A4: mysqlnd_mysqlnd_res_store_result_fetch_data_pub (mysqlnd_result.c:1326)
==26369==    by 0xAF373F: mysqlnd_mysqlnd_res_store_result_pub (mysqlnd_result.c:1443)
==26369==    by 0xA98876: mysqlnd_mysqlnd_conn_data_store_result_pub (mysqlnd_connection.c:1965)
==26369==    by 0x7D98B1: zif_mysqli_query (mysqli_nonapi.c:617)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)
Member

nikic commented Feb 12, 2017

As suspected, when running the test for bug 73800 you added under valgrind, I get memory errors:

==26369== Invalid read of size 1
==26369==    at 0xABBE49: php_mysqlnd_rowp_read_text_protocol_aux (mysqlnd_wireprotocol.c:1703)
==26369==    by 0xABC57F: php_mysqlnd_rowp_read_text_protocol_zval (mysqlnd_wireprotocol.c:1787)
==26369==    by 0xAF0927: mysqlnd_mysqlnd_result_buffered_zval_fetch_row_pub (mysqlnd_result.c:1096)
==26369==    by 0xAF2146: mysqlnd_mysqlnd_res_fetch_row_pub (mysqlnd_result.c:1275)
==26369==    by 0xAF6BFD: mysqlnd_mysqlnd_res_fetch_into_pub (mysqlnd_result.c:1754)
==26369==    by 0x7C80A6: php_mysqli_fetch_into_hash_aux (mysqli.c:1220)
==26369==    by 0x7C843E: php_mysqli_fetch_into_hash (mysqli.c:1271)
==26369==    by 0x7D836B: zif_mysqli_fetch_array (mysqli_nonapi.c:350)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)
==26369==    by 0xC74825: execute_ex (zend_vm_execute.h:432)
==26369==    by 0xC74AC4: zend_execute (zend_vm_execute.h:474)
==26369==    by 0xC05E7A: zend_execute_scripts (zend.c:1544)
==26369==  Address 0x11d9d8dc is 0 bytes after a block of size 18,000,028 alloc'd
==26369==    at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==26369==    by 0xBC29E5: __zend_realloc (zend_alloc.c:2836)
==26369==    by 0xBC1A1F: _erealloc (zend_alloc.c:2443)
==26369==    by 0xAA2A22: _mysqlnd_erealloc (mysqlnd_alloc.c:267)
==26369==    by 0xB10147: mysqlnd_mempool_resize_chunk (mysqlnd_block_alloc.c:94)
==26369==    by 0xAB9B4A: php_mysqlnd_read_row_ex (mysqlnd_wireprotocol.c:1511)
==26369==    by 0xABCCFD: php_mysqlnd_rowp_read (mysqlnd_wireprotocol.c:1826)
==26369==    by 0xAF26A4: mysqlnd_mysqlnd_res_store_result_fetch_data_pub (mysqlnd_result.c:1326)
==26369==    by 0xAF373F: mysqlnd_mysqlnd_res_store_result_pub (mysqlnd_result.c:1443)
==26369==    by 0xA98876: mysqlnd_mysqlnd_conn_data_store_result_pub (mysqlnd_connection.c:1965)
==26369==    by 0x7D98B1: zif_mysqli_query (mysqli_nonapi.c:617)
==26369==    by 0xC76D40: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1079)
@nikic

This comment has been minimized.

Show comment
Hide comment
@nikic

nikic Feb 12, 2017

Member

The problem with the original patch appears to be that the realloc case is not handled correctly -- we pretend like the extra byte is part of the data there.

Member

nikic commented Feb 12, 2017

The problem with the original patch appears to be that the realloc case is not handled correctly -- we pretend like the extra byte is part of the data there.

@nikic

This comment has been minimized.

Show comment
Hide comment
@nikic

nikic Feb 12, 2017

Member

Merged via 01c1afa with a different fix. Now leaving data_size alone and instead only adding the extra byte when performing the actual alloc/realloc.

Member

nikic commented Feb 12, 2017

Merged via 01c1afa with a different fix. Now leaving data_size alone and instead only adding the extra byte when performing the actual alloc/realloc.

@nikic nikic closed this Feb 12, 2017

@nikic nikic removed their request for review Feb 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment