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

Buffer overflow in dict_index_t::col_info::add()

Details

    Description

      The following change to a regression test will cause a crash:

      diff --git a/mysql-test/suite/innodb/t/innodb-index-online.test b/mysql-test/suite/innodb/t/innodb-index-online.test
      		 
      index 2cb84b18402..2f5926d3a53 100644
      		 
      --- a/mysql-test/suite/innodb/t/innodb-index-online.test
      		 
      +++ b/mysql-test/suite/innodb/t/innodb-index-online.test
      		 
      @@ -15,7 +15,8 @@ SET GLOBAL innodb_file_per_table = on;
      		 
       # Save the initial number of concurrent sessions.
      		 
       --source include/count_sessions.inc
      		 
       
      -CREATE TABLE t1 (c1 INT PRIMARY KEY, c2 INT, c3 TEXT)
      		 
      +CREATE TABLE t1 (c1 INT PRIMARY KEY, c2 INT,
      		 
      +c3 VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_bin, INDEX c2c3(c2,c3))
      		 
       ENGINE=InnoDB STATS_PERSISTENT=0;
      		 
       INSERT INTO t1 VALUES (1,1,''), (2,2,''), (3,3,''), (4,4,''), (5,5,'');
      		 
       
      @@ -377,10 +378,11 @@ CHECK TABLE t1;
      		 
       --error ER_DUP_ENTRY
      		 
       ALTER TABLE t1 ADD UNIQUE INDEX c3p5(c3(5));
      		 
       UPDATE t1 SET c3 = NULL WHERE c3 = '';
      		 
      +ALTER TABLE t1 ADD UNIQUE INDEX c3p5(c3(5));
      		 
       SET lock_wait_timeout = 1;
      		 
       SET DEBUG_SYNC = 'row_log_apply_before SIGNAL c3p5_created WAIT_FOR ins_done';
      		 
       --send
      		 
      -ALTER TABLE t1 ADD UNIQUE INDEX c3p5(c3(5));
      		 
      +ALTER TABLE t1 MODIFY c3 VARCHAR(10) CHARACTER SET utf8 COLLATE utf8_unicode_ci;
      		 
       
       connection default;
      		 
       SET DEBUG_SYNC = 'now WAIT_FOR c3p5_created';

      10.6 f410974f0f38999a08ad0d3f9c109ee184db7f31

      		 
      CURRENT_TEST: innodb.innodb-index-online
      		 
      mysqltest: At line 401: query 'reap' failed with wrong errno <Unknown> (2013): 'Lost connection to server during query', instead of ER_LOCK_WAIT_TIMEOUT (1205)...
      		 
      …
      		 
      mariadbd: /mariadb/10.6/storage/innobase/include/dict0mem.h:980: dict_col_t* dict_index_t::col_info::add(mem_heap_t*, const dict_col_t&, unsigned int): Assertion `offset < n_cols' failed.

      The ha_innobase_inplace_ctx::cleanup_col_collation() is supposed to replace the column metadata for the second indexed column of the index c2c3. Instead, it is replacing the metadata for the first (unchanged) column c2 and would then run out of elements when replacing c3.

      When I attempted to retain the TEXT type for the column, the test would hang. I did not investigate the reason.

      Attachments

        Issue Links

          is caused by

          Bug - A problem which impairs or prevents the functions of the product. MDEV-26294 Duplicate entries in unique index not detected when changing collation with INPLACE / NOCOPY algoritm

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed
          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-29436 Change text column collation unnecessarily requires ALGORITHM=COPY

          • Major - Major loss of function.
          • Open

          Activity

            Ascending order - Click to sort in descending order
            marko Marko Mäkelä added a comment -

            thiru, do you think that the following would fix this? That is, we would only attempt to replace temporarily allocated column definitions?

            diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc
            		 
            index 3edb19c8e61..85d2a86e271 100644
            		 
            --- a/storage/innobase/handler/handler0alter.cc
            		 
            +++ b/storage/innobase/handler/handler0alter.cc
            		 
            @@ -1444,7 +1444,9 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx
            		 
                   for (unsigned i= 0, j= 0; i < index->n_fields; i++)
            		 
                   {
            		 
                     const dict_col_t *col= index->fields[i].col;
            		 
            -        if (change_col_collate.find(col->ind) == collate_end)
            		 
            +        if (col >= old_table->cols &&
            		 
            +            col < old_table->cols + old_table->n_cols);
            		 
            +        else if (change_col_collate.find(col->ind) == collate_end)
            		 
                       index->fields[i].col=
            		 
                         index->change_col_info->add(index->heap, *col, j++);
            		 
                   }

            I am wondering if we could eliminate the change_col_collate map altogether.

            marko Marko Mäkelä added a comment - thiru , do you think that the following would fix this? That is, we would only attempt to replace temporarily allocated column definitions? diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 3edb19c8e61..85d2a86e271 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -1444,7 +1444,9 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx for (unsigned i= 0, j= 0; i < index->n_fields; i++) { const dict_col_t *col= index->fields[i].col; - if (change_col_collate.find(col->ind) == collate_end) + if (col >= old_table->cols && + col < old_table->cols + old_table->n_cols); + else if (change_col_collate.find(col->ind) == collate_end) index->fields[i].col= index->change_col_info->add(index->heap, *col, j++); } I am wondering if we could eliminate the change_col_collate map altogether.
            marko Marko Mäkelä added a comment -

            The anomaly with a TEXT column was filed as MDEV-29436.

            marko Marko Mäkelä added a comment - The anomaly with a TEXT column was filed as MDEV-29436 .

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              1 Vote for this issue
              Watchers:
              3 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.