[MDEV-8474] InnoDB sets per-connection data unsafely - JIRA

Details

Type: Bug
Status: Closed
Priority: Major
Resolution: Fixed
Affects Version/s: 10.1, 10.0, 5.5
Fix Version/s: 10.0.21, 5.5.45
Component/s: Storage Engine - InnoDB, Storage Engine - XtraDB
Labels:
None

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.

Gliffy Diagrams

Attachments

Issue Links

relates to

MDEV-7914 spider/bg.ha fails sporadically in buildbot

In Progress

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Jan Lindström added a comment - 2015-07-21 09:45

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

Show

Jan Lindström added a comment - 2015-07-21 09:45 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

Hide

Permalink

Sergey Vojtovich added a comment - 2015-07-21 11:01

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.

Show

Sergey Vojtovich added a comment - 2015-07-21 11:01 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.

Hide

Permalink

Sergey Vojtovich added a comment - 2015-07-21 11:30

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;

Show

Sergey Vojtovich added a comment - 2015-07-21 11:30 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;

Hide

Permalink

Sergey Vojtovich added a comment - 2015-07-21 11:48

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;

Show

Sergey Vojtovich added a comment - 2015-07-21 11:48 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;

Hide

Permalink

Jan Lindström added a comment - 2015-07-21 12:20

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.

Show

Jan Lindström added a comment - 2015-07-21 12:20 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.

InnoDB sets per-connection data unsafely

Details

Description

Gliffy Diagrams

Attachments

Issue Links

Activity

People

Dates

Time Tracking

Agile