From 1f306d395d00df158702d35b3338ccfe8663744e Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Mon, 20 Jan 2025 17:50:29 +1100 Subject: [PATCH] MDEV-34849 Spider: update conn->queue_connect_share when needed Update conn->queue_connect_share in spider_check_trx_and_get_conn to avoid use-after-free. There are two branches in spider_check_trx_and_get_conn, often called at the beginning of a spider DML, depending on whether an update of various spider fields is needed. If it is determined to be needed, the updating may NULL the connections associated with the spider handler, which subsequently causes a call to spider_get_conn() which updates conn->queued_connect_share with the SPIDER_SHARE associated with the spider handler. We make it so that conn->queued_connect_share is updated regardless of the branch it enters, so that it will not be a stale and potentially already freed one. --- .../spider/bugfix/r/mdev_34849.result | 24 +++++++++ .../spider/bugfix/t/mdev_34849.test | 52 +++++++++++++++++++ storage/spider/spd_trx.cc | 2 + 3 files changed, 78 insertions(+) create mode 100644 storage/spider/mysql-test/spider/bugfix/r/mdev_34849.result create mode 100644 storage/spider/mysql-test/spider/bugfix/t/mdev_34849.test diff --git a/storage/spider/mysql-test/spider/bugfix/r/mdev_34849.result b/storage/spider/mysql-test/spider/bugfix/r/mdev_34849.result new file mode 100644 index 00000000000..9433dd7f679 --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/r/mdev_34849.result @@ -0,0 +1,24 @@ +# +# MDEV-34849 SIGSEGV in server_mysql_real_connect, spider_db_connect, __strcmp_evex and __strnlen_evex, ASAN heap-use-after-free in spider_db_connect on INSERT +# +INSTALL SONAME 'ha_spider'; +CREATE TABLE t1 (c INT) ENGINE=Spider; +CREATE TABLE t2 (c INT) ENGINE=Spider; +SELECT * FROM t2; +ERROR HY000: Unable to connect to foreign data source: localhost +set @old_table_open_cache=@@global.table_open_cache; +SET GLOBAL table_open_cache=0; +Warnings: +Warning 1292 Truncated incorrect table_open_cache value: '0' +set autocommit=0; +/* 1 */ INSERT INTO t1 VALUES (0); +ERROR HY000: Unable to connect to foreign data source: localhost +/* 2 */ INSERT INTO t2 VALUES (0); +ERROR HY000: Unable to connect to foreign data source: localhost +set global spider_connect_error_interval=0; +/* 3 */ INSERT INTO t1 VALUES (0); +ERROR HY000: Unable to connect to foreign data source: localhost +drop table t1, t2; +set global table_open_cache=@old_table_open_cache; +Warnings: +Warning 1620 Plugin is busy and will be uninstalled on shutdown diff --git a/storage/spider/mysql-test/spider/bugfix/t/mdev_34849.test b/storage/spider/mysql-test/spider/bugfix/t/mdev_34849.test new file mode 100644 index 00000000000..4890abf4c7d --- /dev/null +++ b/storage/spider/mysql-test/spider/bugfix/t/mdev_34849.test @@ -0,0 +1,52 @@ +--echo # +--echo # MDEV-34849 SIGSEGV in server_mysql_real_connect, spider_db_connect, __strcmp_evex and __strnlen_evex, ASAN heap-use-after-free in spider_db_connect on INSERT +--echo # + +INSTALL SONAME 'ha_spider'; + +CREATE TABLE t1 (c INT) ENGINE=Spider; +CREATE TABLE t2 (c INT) ENGINE=Spider; + +# So that t2 is inserted into spider_init_error_tables and in INSERT +# INTO t2 we go into failure mode in spider_get_share() +--error 1429 +SELECT * FROM t2; + +# Resets the table cache so that the next two queries will call +# ha_spider::open() on t1 and t2 respectively +set @old_table_open_cache=@@global.table_open_cache; +SET GLOBAL table_open_cache=0; + +# This causes trx_ha->wait_for_reusing to remain false during the +# (non-)rollback at the end of the first INSERT INTO t1 statement, so +# that the second INSERT INTO t1 enters the branch in +# spider_check_trx_and_get_conn() that does not update spider fields +# including NULLing its associated connections. +set autocommit=0; + +# Misses the table cache when opening the table. Spider then opens the +# table so that the next INSERT INTO t1 causes a table cache hit and +# skips the call to open table with spider +--error ER_CONNECT_TO_FOREIGN_DATA_SOURCE +/* 1 */ INSERT INTO t1 VALUES (0); + +# Spider opens the table and creates a t2 share, assigns it to +# conn->queued_connect_share, and frees the t2 share on failure +--error ER_CONNECT_TO_FOREIGN_DATA_SOURCE +/* 2 */ INSERT INTO t2 VALUES (0); + +# So that the final INSERT INTO t1 will decide not to return the same +# error in spider_db_connect(), and move onto using the freed share +set global spider_connect_error_interval=0; + +# Skips call to ha_spider::open(), so it does not create a t1 share +# nor reassign it to conn->queued_connect_share, causing it to remain +# the freed t2 share, and using the share results in segv +--error ER_CONNECT_TO_FOREIGN_DATA_SOURCE +/* 3 */ INSERT INTO t1 VALUES (0); + +drop table t1, t2; + +set global table_open_cache=@old_table_open_cache; +--disable_query_log +--source ../../include/clean_up_spider.inc diff --git a/storage/spider/spd_trx.cc b/storage/spider/spd_trx.cc index 290b1fb1127..c52c781af8a 100644 --- a/storage/spider/spd_trx.cc +++ b/storage/spider/spd_trx.cc @@ -3510,6 +3510,8 @@ static int spider_trx_get_conn(ha_spider *spider, SPIDER_TRX *trx, /* TODO: do we need the check for !from_if here? */ if (!from_if) conn->error_mode&= spider->error_mode; + if (conn->queued_connect) + conn->queued_connect_share= share; } else if (!(conn = spider_get_conn(share, roop_count,