Skip to content

PDO OCI segfault in statement GC #18494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mvorisek opened this issue May 4, 2025 · 8 comments
Closed

PDO OCI segfault in statement GC #18494

mvorisek opened this issue May 4, 2025 · 8 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented May 4, 2025

Description

Backtrace:

$gdb -batch -ex run -ex bt --args php vendor/bin/phpunit --exclude-group none --no-coverage --fail-on-warning --fail-on-risky $(if vendor/bin/phpunit --version | grep -q '^PHPUnit 9\.'; then echo -v; else echo --fail-on-notice --fail-on-deprecation --display-notices --display-deprecations --display-warnings --display-errors --display-incomplete --display-skipped; fi)
  env:
    DB_DSN: pdo_oci:dbname=oracle/free
    DB_USER: system
    DB_PASSWORD: atk4_pass
    NLS_LANG: AMERICAN_AMERICA.AL32UTF8

warning: Error disabling address space randomization: Operation not permitted
PHPUnit 11.5.19 by Sebastian Bergmann and contributors.
Runtime:       PHP 8.3.20
Configuration: /__w/data/data/phpunit.xml.dist
Program received signal SIGSEGV, Segmentation fault.
0x00007fcc20f99908 in kpulitmp () from /usr/lib/oracle/21.1/client64/lib/libclntsh.so.21.1
#0  0x00007fcc20f99908 in kpulitmp () from /usr/lib/oracle/21.1/client64/lib/libclntsh.so.21.1
#1  0x00007fcc1d934f1f in oci_stmt_dtor () from /usr/local/lib/php/extensions/debug-non-zts-20230831/pdo_oci.so
#2  0x0000561ba909aaa9 in php_pdo_free_statement (stmt=0x7fcc1b9831c0) at /usr/src/php/ext/pdo/pdo_stmt.c:2139
#3  0x0000561ba909aba6 in pdo_dbstmt_free_storage (std=0x7fcc1b9832f8) at /usr/src/php/ext/pdo/pdo_stmt.c:2166
#4  0x0000561ba94ae726 in zend_gc_collect_cycles () at /usr/src/php/Zend/zend_gc.c:1938
#5  0x0000561ba93efb68 in zif_gc_collect_cycles (execute_data=0x7fcc25e1ac40, return_value=0x7ffdf03cbd50) at /usr/src/php/Zend/zend_builtin_functions.c:93
#6  0x0000561ba9412502 in ZEND_DO_FCALL_BY_NAME_SPEC_RETVAL_UNUSED_HANDLER () at /usr/src/php/Zend/zend_vm_execute.h:1567
#7  0x0000561ba9491d28 in execute_ex (ex=0x7fcc25e17020) at /usr/src/php/Zend/zend_vm_execute.h:57266
#8  0x0000561ba9496646 in zend_execute (op_array=0x7fcc25e61140, return_value=0x0) at /usr/src/php/Zend/zend_vm_execute.h:61634
#9  0x0000561ba93cc203 in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /usr/src/php/Zend/zend.c:1895
#10 0x0000561ba9310660 in php_execute_script (primary_file=0x7ffdf03cd540) at /usr/src/php/main/main.c:2529
#11 0x0000561ba955a259 in do_cli (argc=15, argv=0x7fcc264d9b20) at /usr/src/php/sapi/cli/php_cli.c:966
#12 0x0000561ba955b0a3 in main (argc=15, argv=0x7fcc264d9b20) at /usr/src/php/sapi/cli/php_cli.c:1341

Full repro code: https://github.com/atk4/data/tree/repro_oracle_83_segfault

Notes:

I tried to isolate the reproducer into a single file /wo phpunit, but it seems even merging the 2 tests into 1 prevent this bug.

Also this issue seems to be present with PDO OCI driver only. OCI8 and other PDO drivers are unaffected.

PHP Version

8.3.20

Operating System

Alpine

@nielsdos
Copy link
Member

nielsdos commented May 4, 2025

I recognize this type of issue I think based on the trace, I fixed some crap like this before: 2ae897f
Don't have OCI on here to check though.

@mvorisek
Copy link
Contributor Author

mvorisek commented May 4, 2025

Thanks.

The issue is reproducible at least with the bundled pdo_oci and https://github.com/atk4/data/blob/6.0.0/.github/workflows/test-unit.yml#L115-L118 docker Oracle server.

@nielsdos do you think we can somehow work around this in php code?

@nielsdos
Copy link
Member

nielsdos commented May 4, 2025

Only if you disable GC.
I'll check how hard it is to install the oracle's instant client on my machine... It's not in my package manager unfortunately so I'll have to build myself.

@nielsdos
Copy link
Member

nielsdos commented May 4, 2025

Are you able to do a build of PHP yourself with this patch applied?

diff --git a/ext/pdo_oci/oci_statement.c b/ext/pdo_oci/oci_statement.c
index f3be69f9c32..f91b27975e8 100644
--- a/ext/pdo_oci/oci_statement.c
+++ b/ext/pdo_oci/oci_statement.c
@@ -98,14 +98,21 @@ static int oci_stmt_dtor(pdo_stmt_t *stmt) /* {{{ */
 		S->einfo.errmsg = NULL;
 	}
 
+	/* TODO: There's php_pdo_stmt_valid_db_obj_handle in PHP-8.5-dev that does these checks. */
+	bool server_obj_usable = !Z_ISUNDEF(stmt->database_object_handle)
+		&& IS_OBJ_VALID(EG(objects_store).object_buckets[Z_OBJ_HANDLE(stmt->database_object_handle)])
+		&& !(OBJ_FLAGS(Z_OBJ(stmt->database_object_handle)) & IS_OBJ_FREE_CALLED);
+
 	if (S->cols) {
 		for (i = 0; i < stmt->column_count; i++) {
 			if (S->cols[i].data) {
 				switch (S->cols[i].dtype) {
 					case SQLT_BLOB:
 					case SQLT_CLOB:
-						OCI_TEMPLOB_CLOSE(S->H->env, S->H->svc, S->H->err,
-							(OCILobLocator *) S->cols[i].data);
+						if (server_obj_usable) {
+							OCI_TEMPLOB_CLOSE(S->H->env, S->H->svc, S->H->err,
+								(OCILobLocator *) S->cols[i].data);
+						}
 						OCIDescriptorFree(S->cols[i].data, OCI_DTYPE_LOB);
 						break;
 					default:

@mvorisek
Copy link
Contributor Author

mvorisek commented May 4, 2025

I am trying to test this on my side. I need to tweak our php image build process first.

Is it only a coincidence that this issue is present for PHP 8.3+ only or is it present also for PHP 8.2?

@mvorisek
Copy link
Contributor Author

mvorisek commented May 4, 2025

🏆 I can confirm the issue is gone with the patch above!

@nielsdos
Copy link
Member

nielsdos commented May 4, 2025

Thanks for testing, I'll make a PR, I'm not going to isolate a test however as that'll take me too long and I only have the OCI setup for building. Besides, it's similar to the other patches.
The only annoying problem is that I'll also have to submit this to the PECL repository, and that repo has little activity...

@nielsdos nielsdos changed the title PDO OCI segfault since PHP 8.3 PDO OCI segfault in statement GC May 4, 2025
nielsdos added a commit to nielsdos/php-src that referenced this issue May 4, 2025
This is the same issue that was fixed in 2ae897f, but now for OCI.
@nielsdos
Copy link
Member

nielsdos commented May 4, 2025

Is it only a coincidence that this issue is present for PHP 8.3+ only or is it present also for PHP 8.2?

I believe this also applies on 8.2.

@nielsdos nielsdos linked a pull request May 4, 2025 that will close this issue
nielsdos added a commit that referenced this issue May 5, 2025
* PHP-8.3:
  Fix GH-18494: PDO OCI segfault in statement GC
nielsdos added a commit that referenced this issue May 5, 2025
* PHP-8.4:
  Fix GH-18494: PDO OCI segfault in statement GC
nielsdos added a commit to nielsdos/pecl-database-pdo_oci that referenced this issue May 5, 2025
This is the same issue that was fixed in 2ae897fff7af3a794a31a8aeeeeb4f21f6a41393, but now for OCI.

Fixed at php-src on 8.3 in php/php-src#18495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants