Skip to content
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

libphp.so: oci8.php_oci_cleanup_global_handles segfaults at second call #7765

Closed
lzsiga opened this issue Dec 12, 2021 · 4 comments
Closed

Comments

@lzsiga
Copy link

lzsiga commented Dec 12, 2021

Description

When httpd is terminating, function oci8.php_oci_cleanup_global_handles is sometimes called twice. It symbol ZTS is defined, the second call fails, as the thread specific data-area has already been freed by that time.

Suggested change for php_oci8_int.h
before:

542 #ifdef ZTS
543 #define OCI_G(v) TSRMG(oci_globals_id, zend_oci_globals *, v)
544 #else
545 #define OCI_G(v) (oci_globals.v)
546 #endif

after:

542 #ifdef ZTS
543 # define OCI_GLOBALS TSRMG_BULK(oci_globals_id, zend_oci_globals *)
544 #else
545 # define OCI_GLOBALS (&oci_globals)
546 #endif
547 #define OCI_G(v) (OCI_GLOBALS->v)

Suggested change for oci8.c

before:

246 static void php_oci_cleanup_global_handles(void)
247 {
248     if (OCI_G(err)) {
249         PHP_OCI_CALL(OCIHandleFree, ((dvoid *) OCI_G(err), OCI_HTYPE_ERROR));
250         OCI_G(err) = NULL;
251     }
252
253     if (OCI_G(env)) {
254         PHP_OCI_CALL(OCIHandleFree, ((dvoid *) OCI_G(env), OCI_HTYPE_ENV));
255         OCI_G(env) = NULL;
256     }
257 }

after:

246 static void php_oci_cleanup_global_handles(void)
247 {
248     if (OCI_GLOBALS && OCI_G(err)) {
249         PHP_OCI_CALL(OCIHandleFree, ((dvoid *) OCI_G(err), OCI_HTYPE_ERROR));
250         OCI_G(err) = NULL;
251     }
252
253     if (OCI_GLOBALS && OCI_G(env)) {
254         PHP_OCI_CALL(OCIHandleFree, ((dvoid *) OCI_G(env), OCI_HTYPE_ENV));
255         OCI_G(env) = NULL;
256     }
257 }

or perhaps:

246 static void php_oci_cleanup_global_handles(void)
247 {
248     zend_oci_globals *og= OCI_GLOBALS;
249     if (og) {
250         if (og->err) {
251             PHP_OCI_CALL(OCIHandleFree, ((dvoid *) og->err, OCI_HTYPE_ERROR));
252             og->err = NULL;
253         }
254
255         if (og->env) {
256             PHP_OCI_CALL(OCIHandleFree, ((dvoid *) og->env, OCI_HTYPE_ENV));
257             og->env = NULL;
258         }
259     }
260 }

PHP Version

all with ZTS defined

Operating System

All having pthread

@cmb69
Copy link
Member

cmb69 commented Dec 12, 2021

It seems to me that the problem is the use of the OCI_G macro during GSHUTDOWN, instead of using the passed oci pointer, which would need to be passed to php_oci_cleanup_global_handles().

cmb69 added a commit to cmb69/php-src that referenced this issue Dec 12, 2021
We must not use the TSRM accessor macros in GINIT and GSHUTDOWN, but
rather use the passed pointers directly.  For simplicity, we inline
`php_oci_cleanup_global_handles()`, and also the `PHP_OCI_CALL()`
macros; the latter are unlikely to be needed here, but don't hurt.
@cmb69 cmb69 self-assigned this Dec 12, 2021
@cmb69
Copy link
Member

cmb69 commented Dec 12, 2021

@lzsiga, could you please check whether PR #7766 would fix the issue for you?

cmb69 added a commit that referenced this issue Dec 12, 2021
* PHP-8.0:
  Fix GH-7765: php_oci_cleanup_global_handles segfaults at second call
@cmb69 cmb69 closed this as completed in c435e67 Dec 12, 2021
cmb69 added a commit that referenced this issue Dec 12, 2021
* PHP-8.1:
  Fix GH-7765: php_oci_cleanup_global_handles segfaults at second call
@lzsiga
Copy link
Author

lzsiga commented Dec 13, 2021

@cmb69 Well, for the first glance I cannot find the check for oci_globals!=NULL in the patch but maybe that's not necessary. Anyways, the problem doesn't occur within a quick start-stop sequence, so I'll let it run for a few days, and will report back. Best wishes.

@lzsiga
Copy link
Author

lzsiga commented Dec 16, 2021

So far so good.

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

No branches or pull requests

3 participants
@cmb69 @lzsiga and others