From 0b73ef0688b320c2ff7fe678b28041f3e7505e97 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 16 Sep 2020 18:29:11 +0300 Subject: [PATCH] MDEV-21470 ASAN heap-use-after-free in my_hash_sort_bin The problem was that the server was calling virtual functions on a record that was not initialized with new data. This happened when fill_record() was aborted in the middle because an error in save_val() or save_in_field() --- mysql-test/main/type_blob.result | 15 +++++++++++++++ mysql-test/main/type_blob.test | 20 ++++++++++++++++++++ sql/sql_base.cc | 19 +++++++++++-------- sql/table.cc | 24 +++++++++++++++++------- 4 files changed, 63 insertions(+), 15 deletions(-) diff --git a/mysql-test/main/type_blob.result b/mysql-test/main/type_blob.result index 967fb37b230fc..a0a5f82a65688 100644 --- a/mysql-test/main/type_blob.result +++ b/mysql-test/main/type_blob.result @@ -1125,5 +1125,20 @@ LENGTH(a) LENGTH(DEFAULT(a)) 256 256 DROP TABLE t1; # +# ASAN heap-use-after-free in my_hash_sort_bin or ER_KEY_NOT_FOUND +# upon INSERT into table with long unique blob +# +SET @save_sql_mode=@@sql_mode; +SET SQL_MODE='STRICT_ALL_TABLES'; +CREATE TABLE t1 (a INT, b BLOB) ENGINE=innodb; +INSERT INTO t1 VALUES (1,'foo'),(2,'bar'); +CREATE TABLE t2 (c BIT, d BLOB, UNIQUE(d)) ENGINE=innodb; +INSERT INTO t2 SELECT * FROM t1; +ERROR 22001: Data too long for column 'c' at row 2 +select * from t2; +c d +DROP TABLE t1, t2; +SET @@sql_mode=@save_sql_mode; +# # End of 10.4 test # diff --git a/mysql-test/main/type_blob.test b/mysql-test/main/type_blob.test index 7801280a8d29c..a9b044b53183e 100644 --- a/mysql-test/main/type_blob.test +++ b/mysql-test/main/type_blob.test @@ -1,6 +1,7 @@ # This test can't be run with running server (--extern) as this uses # load_file() on a file in the tree. # +--source include/have_innodb.inc # # Basic cleanup @@ -735,6 +736,25 @@ INSERT INTO t1 VALUES (); SELECT LENGTH(a), LENGTH(DEFAULT(a)) FROM t1; DROP TABLE t1; +--echo # +--echo # ASAN heap-use-after-free in my_hash_sort_bin or ER_KEY_NOT_FOUND +--echo # upon INSERT into table with long unique blob +--echo # + +# Note that this test worked with myisam as it caches blobs differently than +# InnoDB + +SET @save_sql_mode=@@sql_mode; +SET SQL_MODE='STRICT_ALL_TABLES'; + +CREATE TABLE t1 (a INT, b BLOB) ENGINE=innodb; +INSERT INTO t1 VALUES (1,'foo'),(2,'bar'); +CREATE TABLE t2 (c BIT, d BLOB, UNIQUE(d)) ENGINE=innodb; +--error ER_DATA_TOO_LONG +INSERT INTO t2 SELECT * FROM t1; +select * from t2; +DROP TABLE t1, t2; +SET @@sql_mode=@save_sql_mode; --echo # --echo # End of 10.4 test diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 75427f106a4c8..9f9bee0ad1755 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8813,14 +8813,17 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List &values, goto err; field->set_has_explicit_value(); } - /* Update virtual fields */ - thd->abort_on_warning= FALSE; - if (table->versioned()) - table->vers_update_fields(); - if (table->vfield && - table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_WRITE)) - goto err; - thd->abort_on_warning= abort_on_warning_saved; + /* Update virtual fields if there wasn't any errors */ + if (!thd->is_error()) + { + thd->abort_on_warning= FALSE; + if (table->versioned()) + table->vers_update_fields(); + if (table->vfield && + table->update_virtual_fields(table->file, VCOL_UPDATE_FOR_WRITE)) + goto err; + thd->abort_on_warning= abort_on_warning_saved; + } DBUG_RETURN(thd->is_error()); err: diff --git a/sql/table.cc b/sql/table.cc index 6fa2ef51f891c..5d465cf80b510 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -8268,28 +8268,39 @@ class Turn_errors_to_warnings_handler : public Internal_error_handler @details The function computes the values of the virtual columns of the table and stores them in the table record buffer. + This will be done even if is_error() is set either when function was called + or by calculating the virtual function, as most calls to this + function doesn't check the result. We also want to ensure that as many + fields as possible has the right value so that we can optionally + return the partly-faulty-row from a storage engine with a virtual + field that gives an error on storage for an existing row. + + @todo + Ensure that all caller checks the value of this function and + either properly ignores it (and resets the error) or sends the + error forward to the caller. @retval 0 Success @retval - >0 Error occurred when storing a virtual field value + >0 Error occurred when storing a virtual field value or potentially + is_error() was set when function was called. */ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) { DBUG_ENTER("TABLE::update_virtual_fields"); - DBUG_PRINT("enter", ("update_mode: %d", update_mode)); + DBUG_PRINT("enter", ("update_mode: %d is_error: %d", update_mode, + in_use->is_error())); Field **vfield_ptr, *vf; Query_arena backup_arena; Turn_errors_to_warnings_handler Suppress_errors; - int error; bool handler_pushed= 0, update_all_columns= 1; DBUG_ASSERT(vfield); if (h->keyread_enabled()) DBUG_RETURN(0); - error= 0; in_use->set_n_backup_active_arena(expr_arena, &backup_arena); /* When reading or deleting row, ignore errors from virtual columns */ @@ -8312,7 +8323,7 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) } /* Iterate over virtual fields in the table */ - for (vfield_ptr= vfield; *vfield_ptr; vfield_ptr++) + for (vfield_ptr= vfield; *vfield_ptr ; vfield_ptr++) { vf= (*vfield_ptr); Virtual_column_info *vcol_info= vf->vcol_info; @@ -8363,8 +8374,7 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) int field_error __attribute__((unused)) = 0; /* Compute the actual value of the virtual fields */ DBUG_FIX_WRITE_SET(vf); - if (vcol_info->expr->save_in_field(vf, 0)) - field_error= error= 1; + field_error= vcol_info->expr->save_in_field(vf, 0); DBUG_RESTORE_WRITE_SET(vf); DBUG_PRINT("info", ("field '%s' - updated error: %d", vf->field_name.str, field_error));