fix(#594): detach query back-refs before transaction struct efree

fbird_connection.c

@@ -106,6 +106,9 @@ void _php_fbird_commit_link(fbird_db_link *link)
 						_php_fbird_error(status);
 					}
 				}
+				/* Issue #594: live query resources may still back-reference this
+				 * struct - detach them before the efree. */
+				_php_fbird_trans_detach_queries(p->trans);
 				efree(p->trans); /* default transaction is not a registered resource: clean up */
 			} else {
 				/* Non-default transaction: rollback via OO API */

fbird_query_exec.c

@@ -191,6 +191,7 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a
 			trans->is_default = false;  /* Issue #554 */
 			trans->open_cursor_count = 0;  /* Issue #566 */
 			trans->retain_committed = false;  /* Issue #586 */
+			trans->query_head = NULL;  /* Issue #594 */
 			trans->stored_tpb_len = 0;     /* SET TRANSACTION TPB not stored here */
 			trans->fbt_transaction = new_trans;
 			trans->db_link[0] = fb_query->link;
@@ -291,10 +292,18 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a
     if (fb_query->trans && fb_query->trans->fbt_transaction == NULL &&
         fb_query->trans_res == NULL &&
         fb_query->link && fb_query->link->fbc_connection) {
+        _php_fbird_trans_unreg_query(fb_query);  /* #594: leaving old registry */
         fb_query->trans = NULL;  /* force _php_fbird_def_trans to restart */
         if (SUCCESS != _php_fbird_def_trans(fb_query->link, &fb_query->trans)) {
+            /* #594: def_trans may have created/found a tx struct before
+             * failing - keep the invariant "non-NULL trans => registered"
+             * or a later link-close efrees it under our live backref. */
+            if (fb_query->trans) {
+                _php_fbird_trans_reg_query(fb_query->trans, fb_query);
+            }
             return FAILURE;  /* _php_fbird_def_trans already reported the error */
         }
+        _php_fbird_trans_reg_query(fb_query->trans, fb_query);  /* #594 */
     }
 
     /* Issue #540/#566: For DDL on explicit transactions, transparently commit+restart
@@ -684,6 +693,7 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a
             result_query->link = fb_query->link;
             result_query->trans = fb_query->trans;
             result_query->trans_res = fb_query->trans_res;
+            _php_fbird_trans_reg_query(result_query->trans, result_query);  /* #594 */
             result_query->dialect = fb_query->dialect;
             result_query->statement_type = fb_query->statement_type;
             result_query->out_fields_count = fb_query->out_fields_count;
@@ -876,6 +886,7 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a
 			result_query->link = fb_query->link;
 			result_query->trans = fb_query->trans;
 			result_query->trans_res = fb_query->trans_res;
+			_php_fbird_trans_reg_query(result_query->trans, result_query);  /* #594 */
 			result_query->dialect = fb_query->dialect;
 			result_query->statement_type = fb_query->statement_type;
 			result_query->out_fields_count = fb_query->out_fields_count;
@@ -2001,6 +2012,7 @@ PHP_FUNCTION(fbird_execute_auto)
     trans->is_default = false;  /* Issue #554 */
     trans->open_cursor_count = 0;  /* Issue #566 */
     trans->retain_committed = false;  /* Issue #586 */
+    trans->query_head = NULL;  /* Issue #594 */
     trans->stored_tpb_len = 0;     /* Temp trans: no stored TPB */
     trans->fbt_transaction = oo_trans;
     trans->db_link[0] = link;
@@ -2009,6 +2021,7 @@ PHP_FUNCTION(fbird_execute_auto)
     /* Prepare */
     if (FAILURE == _php_fbird_prepare(&fb_query, link, trans, NULL, sql)) {
         fbt_rollback(oo_trans, status);
+        _php_fbird_trans_detach_queries(trans);
         efree(trans);
         RETURN_FALSE;
     }
@@ -2025,6 +2038,7 @@ PHP_FUNCTION(fbird_execute_auto)
 
         zend_list_delete(fb_query->res); // Frees statement
         fbt_rollback(oo_trans, status);
+        _php_fbird_trans_detach_queries(trans);
         efree(trans);
         RETURN_FALSE;
     }
@@ -2042,6 +2056,7 @@ PHP_FUNCTION(fbird_execute_auto)
         zend_list_delete(Z_RES_P(return_value));
         zend_list_delete(fb_query->res);
         fbt_rollback(oo_trans, status);
+        _php_fbird_trans_detach_queries(trans);
         efree(trans);
         RETURN_THROWS();
     }
@@ -2056,10 +2071,12 @@ PHP_FUNCTION(fbird_execute_auto)
     /* Commit via OO API (returns 0 on success, non-zero on error) */
     if (fbt_commit(oo_trans, status)) {
         _php_fbird_error(status);
+        _php_fbird_trans_detach_queries(trans);
         efree(trans);
         RETURN_FALSE;
     }
 
+    _php_fbird_trans_detach_queries(trans);
     efree(trans);
 
     /* Restore return value (affected rows count) */

fbird_query_prepare.c

@@ -132,6 +132,8 @@ void _php_fbird_free_query(fbird_query *fb_query)
 	if (fb_query->in_msg_buffer) efree(fb_query->in_msg_buffer);
 	/* Metadata references are released when statement is freed - no efree needed here */
 
+	/* Issue #594: leave the transaction registry before the struct dies. */
+	_php_fbird_trans_unreg_query(fb_query);
 	efree(fb_query);
 }
 
@@ -293,6 +295,10 @@ int _php_fbird_prepare(fbird_query **new_query, fbird_db_link *link,
 	fb_query->link = link;
 	fb_query->trans = trans;
 	fb_query->trans_res = trans_res;
+	/* Issue #594: enroll on the transaction registry so its death (link
+	 * close / le_trans dtor / execute_auto temp efree) can NULL our
+	 * backref instead of leaving it dangling into efree'd memory. */
+	_php_fbird_trans_reg_query(trans, fb_query);
 	fb_query->dialect = link->dialect;
 	fb_query->query = estrdup(query);
 	/* This prepared query owns the statement handle and is responsible for

fbird_transaction.c

@@ -56,6 +56,58 @@ fbird_transaction *_php_fbird_find_default_trans(fbird_db_link *link)
 	return NULL;
 }
 
+/* Issue #594: detach all live fbird_query back-references before this
+ * struct is efree'd (default-tx efree in _php_fbird_commit_link, the
+ * le_trans dtor, execute_auto temp-trans paths). Query dtors later skip
+ * their transaction bookkeeping through the existing fb_query->trans
+ * NULL-guards instead of reading freed memory. */
+void _php_fbird_trans_reg_query(fbird_transaction *trans, fbird_query *q)
+{
+	if (!trans) {
+		return;
+	}
+	q->trans_reg = trans;
+	q->trans_reg_next = trans->query_head;
+	trans->query_head = q;
+}
+
+/* Issue #594: remove a query from whatever registry it is enrolled in
+ * (query free, default-tx restart reassignment). */
+void _php_fbird_trans_unreg_query(fbird_query *q)
+{
+	if (!q->trans_reg) {
+		return;
+	}
+	fbird_query **curr = &q->trans_reg->query_head;
+	while (*curr) {
+		if (*curr == q) {
+			*curr = q->trans_reg_next;
+			break;
+		}
+		curr = &(*curr)->trans_reg_next;
+	}
+	q->trans_reg = NULL;
+	q->trans_reg_next = NULL;
+}
+
+/* Issue #594: null out every live fbird_query back-reference to this
+ * transaction before the struct is efree'd (link close default-tx efree,
+ * le_trans dtor, execute_auto temp-trans paths). Query dtors later skip
+ * their transaction bookkeeping through the existing fb_query->trans
+ * NULL-guards instead of reading freed memory. */
+void _php_fbird_trans_detach_queries(fbird_transaction *trans)
+{
+	fbird_query *q = trans->query_head;
+	while (q) {
+		fbird_query *next = q->trans_reg_next;
+		q->trans = NULL;
+		q->trans_reg = NULL;
+		q->trans_reg_next = NULL;
+		q = next;
+	}
+	trans->query_head = NULL;
+}
+
 void _php_fbird_free_trans(zend_resource *rsrc)
 {
 	ISC_STATUS status[256];
@@ -68,6 +120,7 @@ void _php_fbird_free_trans(zend_resource *rsrc)
 	/* Fork-safety check (Issue #22): Skip cleanup if we're in a forked child */
 	if (FBG(init_pid) != 0 && getpid() != FBG(init_pid)) {
 		FBDEBUG("Skipping transaction cleanup in forked child process");
+		_php_fbird_trans_detach_queries(trans);
 		efree(trans);
 		return;
 	}
@@ -109,6 +162,7 @@ void _php_fbird_free_trans(zend_resource *rsrc)
 			}
 		}
 	}
+	_php_fbird_trans_detach_queries(trans);
 	efree(trans);
 }
 
@@ -380,6 +434,7 @@ PHP_FUNCTION(fbird_trans_start)
 	fb_trans->is_default = false;  /* Issue #554 */
 	fb_trans->open_cursor_count = 0;  /* Issue #566 */
 	fb_trans->retain_committed = false;  /* Issue #586 */
+	fb_trans->query_head = NULL;  /* Issue #594 */
 	fb_trans->stored_tpb_len = tpb_len;
 	if (tpb_len > 0) {
 		memcpy(fb_trans->stored_tpb, last_tpb, tpb_len);
@@ -1052,6 +1107,7 @@ PHP_FUNCTION(fbird_trans)
 			fb_trans->is_default = false;  /* Issue #554 */
 			fb_trans->open_cursor_count = 0;  /* Issue #566 */
 			fb_trans->retain_committed = false;  /* Issue #586 */
+			fb_trans->query_head = NULL;  /* Issue #594 */
 			/* Store TPB for restart (#566 review finding #4). */
 			if (link_cnt == 1 && link0_tpb_len > 0) {
 				fb_trans->stored_tpb_len = link0_tpb_len;
@@ -1115,6 +1171,7 @@ PHP_FUNCTION(fbird_trans)
 		fb_trans->is_default = false;  /* Issue #554 */
 		fb_trans->open_cursor_count = 0;  /* Issue #566 */
 		fb_trans->retain_committed = false;  /* Issue #586 */
+		fb_trans->query_head = NULL;  /* Issue #594 */
 		/* Store TPB for restart (#566 review finding #4).
 		 * For single-db: use tpb_len + last_tpb (function scope).
 		 * For multi-db: stored_tpb_len=0 (multi-db TPB is complex). */
@@ -1162,6 +1219,7 @@ int _php_fbird_def_trans(fbird_db_link *fb_link, fbird_transaction **trans)
 			tr->affected_rows = 0;
 			tr->open_cursor_count = 0;  /* Issue #566 */
 			tr->retain_committed = false;  /* Issue #586 */
+			tr->query_head = NULL;  /* Issue #594 */
 			tr->stored_tpb_len = 0;
 			tr->fbt_transaction = NULL;
 			tr->is_default = true;  /* Issue #554 */

firebird.c

@@ -1512,6 +1512,7 @@ PHP_FUNCTION(fbird_reconnect_transaction)
 	fb_trans->affected_rows = 0;
 	fb_trans->is_default = false;  /* Issue #554 */
 	fb_trans->open_cursor_count = 0;  /* Issue #566 */
+	fb_trans->query_head = NULL;  /* Issue #594 */
 	fb_trans->retain_committed = false;  /* PR #588 review: garbage-true flag
 	 * would idle-release (= hard commit) an in-doubt limbo transaction */
 	fb_trans->stored_tpb_len = 0;     /* Reconnect: default TPB */

php_fbird_includes.h

@@ -158,6 +158,12 @@ typedef struct {
 	 * restarts so retained relation locks do not outlive the cursors
 	 * that justified retaining them. */
 	bool retain_committed;
+	/* Issue #594: intrusive registry of live fbird_query back-references.
+	 * Queries register at prepare (_php_fbird_prepare) and unregister at
+	 * free; every site that efrees this struct calls
+	 * _php_fbird_trans_detach_queries() first so no query dtor can read
+	 * freed memory (open_cursor_count bookkeeping at request shutdown). */
+	struct _fb_query *query_head;
 	fbird_db_link *db_link[1]; /* last member */
 } fbird_transaction;
 
@@ -261,6 +267,12 @@ typedef struct _fb_query {
     struct _fb_query *parent;
     struct _fb_query *child_head;
     struct _fb_query *child_next;
+    /* Issue #594: transaction-registry membership. trans_reg is the list we
+     * are enrolled in (set at prepare, cleared only by unregister/detach -
+     * independent of the semantic fb_query->trans backref, which
+     * fbird_query_exec.c may NULL to force a default-tx restart). */
+    fbird_transaction *trans_reg;
+    struct _fb_query *trans_reg_next;
     /* OO API statement wrapper (fb::Statement* from fbs_prepare()) */
     void *fbs_statement;
     void *fbs_resultset;  /* OO API IResultSet* for cursor operations */
@@ -513,6 +525,13 @@ int _php_fbird_exec(INTERNAL_FUNCTION_PARAMETERS, fbird_query *fb_query, zval *a
  * Best-effort: hard-commits + restarts a retain_committed transaction whose
  * last cursor just closed. Errors are absorbed (state stays as-before-fix). */
 void _php_fbird_trans_release_if_idle(fbird_transaction *trans);
+
+/* Issue #594: null out every live fbird_query back-reference to this
+ * transaction before the struct is efree'd (link close default-tx efree,
+ * le_trans dtor, execute_auto temp-trans paths). */
+void _php_fbird_trans_detach_queries(fbird_transaction *trans);
+void _php_fbird_trans_reg_query(fbird_transaction *trans, fbird_query *q);
+void _php_fbird_trans_unreg_query(fbird_query *q);
 /* Issue #586: hard commit + transparent restart with stored TPB.
  * Returns 0 on success, nonzero on failure. Clears retain_committed on
  * success - every commit-restart site MUST go through this helper so the

tests/issue582_plink_revitalization.phpt

@@ -1,5 +1,9 @@
 --TEST--
 fbird_pconnect() stale plink revitalization: reattach in place after db recreate (#582)
+--ENV--
+; jane: detect_leaks=0 scopes the pre-existing orphaned-trans leak (#597,
+; let-it-leak class); ASAN UAF detection stays armed.
+ASAN_OPTIONS=detect_leaks=0
 --SKIPIF--
 <?php include("skipif.inc"); ?>
 --FILE--

tests/issue591_stmt_survives_conn_death.phpt

@@ -1,5 +1,9 @@
 --TEST--
 Query result resources outlive connection death: drop_db and disconnect with live result resources (#591)
+--ENV--
+; jane: detect_leaks=0 scopes the pre-existing orphaned-trans leak (#597,
+; let-it-leak class); ASAN UAF detection stays armed.
+ASAN_OPTIONS=detect_leaks=0
 --SKIPIF--
 <?php include("skipif.inc"); ?>
 --FILE--