[MDEV-8474] InnoDB sets per-connection data unsafely Created: 2015-07-16  Updated: 2015-07-21  Resolved: 2015-07-21

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB, Storage Engine - XtraDB
Affects Version/s: 5.5, 10.0, 10.1
Fix Version/s: 5.5.45, 10.0.21

Type: Bug Priority: Major
Reporter: Sergey Vojtovich Assignee: Jan Lindström (Inactive)
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Relates
relates to MDEV-7914 spider/bg.ha, spider/bg.ha_part crash... Closed

 Description   

InnoDB sets THD::ha_data as following:

static inline
 
trx_t*&
 
thd_to_trx(
 
/*=======*/
 
        THD*    thd)    /*!< in: MySQL thread */
 
{
 
        return(*(trx_t**) thd_ha_data(thd, innodb_hton_ptr));
 
}
 
 
 
static inline
 
trx_t*
 
check_trx_exists(
 
/*=============*/
 
        THD*    thd)    /*!< in: user thread handle */
 
{
 
        trx_t*& trx = thd_to_trx(thd);
 
 
 
        if (trx == NULL) {
 
                trx = innobase_trx_allocate(thd);
 
        } else if (UNIV_UNLIKELY(trx->magic_n != TRX_MAGIC_N)) {
 
                mem_analyze_corruption(trx);
 
                ut_error;
 
        }
 
 
 
        innobase_trx_init(thd, trx);
 
 
 
        return(trx);
 
}

This is unsafe, because nothing prevents InnoDB plugin from being uninstalled while there's active transaction. This can cause crashes and any other odd behavior. It may also corrupt stack, as functions pointers are not available after dlclose. E.g. spider has similar bug and outcome was like MDEV-7914.

To reproduce this it should be enough to have one thread with active InnoDB transaction, no InnoDB tables in table cache and one thread issuing UNINSTALL PLUGIN innodb.

The fix is to use thd_set_ha_data() when manipulating per-connection handler data. It does appropriate plugin locking.

 Comments   
Comment by Jan Lindström (Inactive) [ 2015-07-21 ]

Hi, I could not repeat with

--source include/not_embedded.inc
 
 
 
if (!$HA_INNODB_SO) {
 
  --skip Need InnoDB plugin
 
}
 
 
 
connect (con1, localhost, root);
 
 
 
connection default;
 
install plugin innodb soname 'ha_innodb';
 
send begin;
 
 
 
connection con1;
 
uninstall plugin innodb;
 
 
 
connection default;
 
reap;
 
create table t1(a int not null primary key) engine=innodb;
 
insert into t1 values (1);
 
drop table t1;
 
disconnect con1;

.opt:

--ignore-builtin-innodb
 
--loose-innodb

I see only:

worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 16000..16019
 
install plugin innodb soname 'ha_innodb';
 
begin;
 
uninstall plugin innodb;
 
create table t1(a int not null primary key) engine=innodb;
 
Warnings:
 
Warning	1286	Unknown storage engine 'innodb'
 
Warning	1266	Using storage engine MyISAM for table 't1'
 
insert into t1 values (1);
 
drop table t1;
 
innodb.innodb_uninstall                  [ pass ]   3576

Comment by Sergey Vojtovich [ 2015-07-21 ]

The following test causes UNINSTALL PLUGIN to wait till all transactions are completed:

--source include/not_embedded.inc
 
 
 
if (!$HA_INNODB_SO) {
 
  --skip Need InnoDB plugin
 
}
 
 
 
install plugin innodb soname 'ha_innodb';
 
create table t1(a int not null primary key) engine=innodb;
 
 
 
connect (con1, localhost, root);
 
connection con1;
 
begin;
 
insert into t1 values(1);
 
 
 
connection default;
 
flush tables;
 
uninstall plugin innodb;
 
drop table t1;
 
disconnect con1;

It's not that bad as such, but it doesn't go inline with normal UNINSTALL PLUGIN at least. Trying to find more serious edge cases now.

Comment by Sergey Vojtovich [ 2015-07-21 ]

The following UNINSTALL PLUGIN seem to have hung forever:

--source include/not_embedded.inc
 
 
 
if (!$HA_INNODB_SO) {
 
  --skip Need InnoDB plugin
 
}
 
 
 
install plugin innodb soname 'ha_innodb';
 
create table t1(a int not null primary key) engine=innodb;
 
 
 
connect (con1, localhost, root);
 
connection con1;
 
begin;
 
insert into t1 values(1);
 
 
 
connection default;
 
flush tables;
 
send uninstall plugin innodb;
 
 
 
connection con1;
 
select sleep(1);
 
disconnect con1;
 
 
 
connection default;
 
reap;

Comment by Sergey Vojtovich [ 2015-07-21 ]

Simpler hang, now default connection doesn't participate in any transaction:

--source include/not_embedded.inc
 
 
 
if (!$HA_INNODB_SO) {
 
  --skip Need InnoDB plugin
 
}
 
 
 
connect (con1, localhost, root);
 
connection con1;
 
install plugin innodb soname 'ha_innodb';
 
create table t1(a int not null primary key) engine=innodb;
 
insert into t1 values(1);
 
drop table t1;
 
 
 
connection default;
 
send uninstall plugin innodb;
 
 
 
connection con1;
 
select sleep(1);
 
disconnect con1;
 
 
 
connection default;
 
reap;

Comment by Jan Lindström (Inactive) [ 2015-07-21 ]

commit 7a9670218b2d1b5673432ebf4e0f028a7c963494
Author: Jan Lindström <jan.lindstrom@mariadb.com>
Date: Tue Jul 21 12:12:58 2015 +0300

MDEV-8474: InnoDB sets per-connection data unsafely

Analysis: At check_trx_exists function InnoDB allocates
a new trx if no trx is found from thd but this newly
allocated trx is not registered to thd. This is unsafe,
because nothing prevents InnoDB plugin from being uninstalled
while there's active transaction. This can cause crashes, hang
and any other odd behavior. It may also corrupt stack, as
functions pointers are not available after dlclose.

Fix: The fix is to use thd_set_ha_data() when
manipulating per-connection handler data. It does appropriate
plugin locking.

Generated at Thu Feb 08 07:27:25 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.