Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-8474

InnoDB sets per-connection data unsafely

Details

    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.

      Attachments

        Issue Links

          Activity

            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

            jplindst Jan Lindström (Inactive) added a comment - 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

            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.

            svoj Sergey Vojtovich added a comment - 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.

            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;

            svoj Sergey Vojtovich added a comment - 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;

            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;

            svoj Sergey Vojtovich added a comment - 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;

            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.

            jplindst Jan Lindström (Inactive) added a comment - 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.

            People

              jplindst Jan Lindström (Inactive)
              svoj Sergey Vojtovich
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.