php / php/php-src

A crash occurred in the odbc_handle_closer function

Open
#20,726 10 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Bug Extension: pdo_odbc Status: Needs Triage
Dominant language
C
Stars
40.4k
Forks
8.1k
Avg merge
2d 13h
Merged PRs (30d)
96

Description

There is an issue with the order of resource release that needs to be corrected by modifying the code.

https://github.com/php/php-src/blob/833120eb72295fd9a6d0cacc68e85cbbeaa6a535/ext/pdo_odbc/odbc_driver.c#L138

The correct code might be:

static void odbc_handle_closer(pdo_dbh_t *dbh) {
    pdo_odbc_db_handle *H = (pdo_odbc_db_handle *) dbh->driver_data;

    SQLFreeHandle(SQL_HANDLE_ENV, H->env);
    H->env = NULL;

    if (H->dbc != SQL_NULL_HANDLE) {
        SQLEndTran(SQL_HANDLE_DBC, H->dbc, SQL_ROLLBACK);
        SQLDisconnect(H->dbc);
        SQLFreeHandle(SQL_HANDLE_DBC, H->dbc);
        H->dbc = NULL;
    }

    pefree(H, dbh->is_persistent);
    dbh->driver_data = NULL;
}

I don't quite understand the logic of the ODBC PDO driver, please review the code above.

==45879== Invalid read of size 8
==45879==    at 0x1185F941: ??? (in /usr/lib/libodbccr.so.2.0.0)
==45879==    by 0x734F411: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x7351182: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x7359691: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x7B01E10: odbc_handle_closer (odbc_driver.c:139)
==45879==    by 0x73F800E: ??? (in /usr/lib/php84/modules/pdo.so)
==45879==    by 0x5CE98E: zend_objects_store_del (in /usr/bin/php84)
==45879==    by 0x586F32: ??? (in /usr/bin/php84)
==45879==    by 0x534247: execute_ex (in /usr/bin/php84)
==45879==    by 0x52D64B: zend_call_function (in /usr/bin/php84)
==45879==    by 0x799996A: swoole::PHPCoroutine::main_func(void*) (swoole_coroutine.cc:773)
==45879==    by 0x7A2B6C7: operator() (std_function.h:591)
==45879==    by 0x7A2B6C7: swoole::coroutine::Context::context_func(transfer_t) (context.cc:149)
==45879==  Address 0x11079928 is 2,344 bytes inside a block of size 5,256 free'd
==45879==    at 0x48B2ECF: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==45879==    by 0x73788CA: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x73597E7: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x7B01DFB: odbc_handle_closer (odbc_driver.c:136)
==45879==    by 0x73F800E: ??? (in /usr/lib/php84/modules/pdo.so)
==45879==    by 0x5CE98E: zend_objects_store_del (in /usr/bin/php84)
==45879==    by 0x586F32: ??? (in /usr/bin/php84)
==45879==    by 0x534247: execute_ex (in /usr/bin/php84)
==45879==    by 0x52D64B: zend_call_function (in /usr/bin/php84)
==45879==    by 0x799996A: swoole::PHPCoroutine::main_func(void*) (swoole_coroutine.cc:773)
==45879==    by 0x7A2B6C7: operator() (std_function.h:591)
==45879==    by 0x7A2B6C7: swoole::coroutine::Context::context_func(transfer_t) (context.cc:149)
==45879==    by 0x7B1F70E: swoole_make_fcontext (make_x86_64_sysv_elf_gas.S:135)
==45879==  Block was alloc'd at
==45879==    at 0x48B640A: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==45879==    by 0x7378711: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x734A3EE: ??? (in /usr/lib/libodbc.so.2.0.0)
==45879==    by 0x7B022FE: pdo_odbc_handle_factory (odbc_driver.c:498)
==45879==    by 0x73F2A2D: php_pdo_internal_construct_driver (in /usr/lib/php84/modules/pdo.so)
==45879==    by 0x552BD0: ??? (in /usr/bin/php84)
==45879==    by 0x534247: execute_ex (in /usr/bin/php84)
==45879==    by 0x52D64B: zend_call_function (in /usr/bin/php84)
==45879==    by 0x799996A: swoole::PHPCoroutine::main_func(void*) (swoole_coroutine.cc:773)
==45879==    by 0x7A2B6C7: operator() (std_function.h:591)
==45879==    by 0x7A2B6C7: swoole::coroutine::Context::context_func(transfer_t) (context.cc:149)
==45879==    by 0x7B1F70E: swoole_make_fcontext (make_x86_64_sysv_elf_gas.S:135)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in ext/pdo_odbc/odbc_driver.c at odbc_handle_closer around lines 136-139, then inspect pdo_odbc_handle_factory around line 498 and the linked Valgrind trace. Verify the resource-release order against the reported invalid read and confirm the reported scenario no longer accesses freed ODBC state.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, php
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.