Skip to content

Commit

Permalink
MDEV-21470 ASAN heap-use-after-free in my_hash_sort_bin
Browse files Browse the repository at this point in the history
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()
  • Loading branch information
montywi committed Sep 25, 2020
1 parent 895e995 commit 0b73ef0
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 15 deletions.
15 changes: 15 additions & 0 deletions mysql-test/main/type_blob.result
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
20 changes: 20 additions & 0 deletions mysql-test/main/type_blob.test
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 11 additions & 8 deletions sql/sql_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8813,14 +8813,17 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &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:
Expand Down
24 changes: 17 additions & 7 deletions sql/table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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;
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 0b73ef0

Please sign in to comment.