From 1c9f64e54ffb109bb6cf6a189e863bfa54e46510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Apr 2025 15:55:45 +0300 Subject: [PATCH 1/8] MDEV-36613 Incorrect undo logging for indexes on virtual columns Starting with mysql/mysql-server@02f8eaa9988dadb73dd68630dd82393cfa19bfb8 and commit 2e814d4702d71a04388386a9f591d14a35980bfe the index ID of indexes on virtual columns was being encoded insufficiently in InnoDB undo log records. Only the least significant 32 bits were being written. This could lead to some corruption of the affected indexes on ROLLBACK, as well as to missed chances to remove some history from such indexes when purging the history of committed transactions that included DELETE or an UPDATE in the indexes. dict_hdr_create(): In debug instrumented builds, initialize the DICT_HDR_INDEX_ID close to the 32-bit barrier, instead of initializing it to DICT_HDR_FIRST_ID (10). This will allow the changed code to be exercised while running ./mtr --suite=gcol,vcol. trx_undo_log_v_idx(): Encode large index->id in a similar way as mysql/mysql-server@e00328b4d068c7485ac2ffe27207ed1f462c718d but using a different implementation. trx_undo_read_v_idx_low(): Decode large index->id in a similar way as mach_u64_read_much_compressed(). Reviewed by: Debarun Banerjee --- .../suite/gcol/r/innodb_virtual_basic.result | 2 + .../suite/gcol/t/innodb_virtual_basic.test | 37 ++++++++++++++++++- storage/innobase/trx/trx0rec.cc | 26 ++++++++++--- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/mysql-test/suite/gcol/r/innodb_virtual_basic.result b/mysql-test/suite/gcol/r/innodb_virtual_basic.result index 3823887186b..35534d68e63 100644 --- a/mysql-test/suite/gcol/r/innodb_virtual_basic.result +++ b/mysql-test/suite/gcol/r/innodb_virtual_basic.result @@ -86,6 +86,8 @@ delete from t where a =13; DROP INDEX idx1 ON t; DROP INDEX idx2 ON t; DROP TABLE t; +# restart +set default_storage_engine=innodb; /* Test large BLOB data */ CREATE TABLE `t` ( `a` BLOB, diff --git a/mysql-test/suite/gcol/t/innodb_virtual_basic.test b/mysql-test/suite/gcol/t/innodb_virtual_basic.test index b64daa2bcdb..69f9f89ccee 100644 --- a/mysql-test/suite/gcol/t/innodb_virtual_basic.test +++ b/mysql-test/suite/gcol/t/innodb_virtual_basic.test @@ -1,6 +1,6 @@ --source include/have_innodb.inc --source include/have_partition.inc ---source include/big_test.inc +--source include/not_embedded.inc call mtr.add_suppression("\\[Warning\\] InnoDB: Compute virtual"); @@ -66,6 +66,41 @@ DROP INDEX idx1 ON t; DROP INDEX idx2 ON t; DROP TABLE t; +let MYSQLD_DATADIR=`select @@datadir`; +let PAGE_SIZE=`select @@innodb_page_size`; +--source include/shutdown_mysqld.inc +perl; +do "$ENV{MTR_SUITE_DIR}/../innodb/include/crc32.pl"; +my $file = "$ENV{MYSQLD_DATADIR}/ibdata1"; +open(FILE, "+<$file") || die "Unable to open $file"; +binmode FILE; +my $ps= $ENV{PAGE_SIZE}; +my $page; +die "Unable to read $file" unless sysread(FILE, $page, $ps) == $ps; +my $full_crc32 = unpack("N",substr($page,54,4)) & 0x10; # FIL_SPACE_FLAGS +sysseek(FILE, 7*$ps, 0) || die "Unable to seek $file\n"; +die "Unable to read $file" unless sysread(FILE, $page, $ps) == $ps; +substr($page,54,4)=pack("N",0xc001cafe); # 32 MSB of 64-bit DICT_HDR_INDEX_ID +my $polynomial = 0x82f63b78; # CRC-32C +if ($full_crc32) +{ + my $ck = mycrc32(substr($page, 0, $ps-4), 0, $polynomial); + substr($page, $ps-4, 4) = pack("N", $ck); +} +else +{ + my $ck= pack("N",mycrc32(substr($page, 4, 22), 0, $polynomial) ^ + mycrc32(substr($page, 38, $ps - 38 - 8), 0, $polynomial)); + substr($page,0,4)=$ck; + substr($page,$ps-8,4)=$ck; +} +sysseek(FILE, 7*$ps, 0) || die "Unable to rewind $file\n"; +syswrite(FILE, $page, $ps)==$ps || die "Unable to write $file\n"; +close(FILE) || die "Unable to close $file"; +EOF +--source include/start_mysqld.inc +set default_storage_engine=innodb; + /* Test large BLOB data */ CREATE TABLE `t` ( `a` BLOB, diff --git a/storage/innobase/trx/trx0rec.cc b/storage/innobase/trx/trx0rec.cc index 33a3962047f..d815f180aba 100644 --- a/storage/innobase/trx/trx0rec.cc +++ b/storage/innobase/trx/trx0rec.cc @@ -148,7 +148,9 @@ trx_undo_log_v_idx( ulint n_idx = 0; for (const auto& v_index : vcol->v_indexes) { n_idx++; - /* FIXME: index->id is 64 bits! */ + if (uint32_t hi= uint32_t(v_index.index->id >> 32)) { + size += 1 + mach_get_compressed_size(hi); + } size += mach_get_compressed_size(uint32_t(v_index.index->id)); size += mach_get_compressed_size(v_index.nth_field); } @@ -175,10 +177,14 @@ trx_undo_log_v_idx( ptr += mach_write_compressed(ptr, n_idx); for (const auto& v_index : vcol->v_indexes) { - ptr += mach_write_compressed( - /* FIXME: index->id is 64 bits! */ - ptr, uint32_t(v_index.index->id)); - + /* This is compatible with + ptr += mach_u64_write_much_compressed(ptr, v_index.index-id) + (the added "if" statement is fixing an old regression). */ + if (uint32_t hi= uint32_t(v_index.index->id >> 32)) { + *ptr++ = 0xff; + ptr += mach_write_compressed(ptr, hi); + } + ptr += mach_write_compressed(ptr, uint32_t(v_index.index->id)); ptr += mach_write_compressed(ptr, v_index.nth_field); } @@ -217,7 +223,15 @@ trx_undo_read_v_idx_low( dict_index_t* clust_index = dict_table_get_first_index(table); for (ulint i = 0; i < num_idx; i++) { - index_id_t id = mach_read_next_compressed(&ptr); + index_id_t id = 0; + /* This is like mach_u64_read_much_compressed(), + but advancing ptr to the next field. */ + if (*ptr == 0xff) { + ptr++; + id = mach_read_next_compressed(&ptr); + id <<= 32; + } + id |= mach_read_next_compressed(&ptr); ulint pos = mach_read_next_compressed(&ptr); dict_index_t* index = dict_table_get_next_index(clust_index); From f89f8aa313cb891b488a0911d9f7e90ad27d0c02 Mon Sep 17 00:00:00 2001 From: Yuchen Pei Date: Tue, 15 Apr 2025 15:49:25 +1000 Subject: [PATCH 2/8] MDEV-36357 MDEV-35452 Temporarily disable view protocol for a query in spider/bg.basic_sql It should be re-enabled once MDEV-36357 is fixed. Also added some documentation to spider result operations. --- storage/spider/ha_spider.cc | 4 ---- .../spider/mysql-test/spider/bg/t/basic_sql.test | 3 +++ storage/spider/spd_db_conn.cc | 14 +++++++++++++- storage/spider/spd_db_include.h | 4 ++-- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/storage/spider/ha_spider.cc b/storage/spider/ha_spider.cc index 0089b1fa0f6..cff2d580f39 100644 --- a/storage/spider/ha_spider.cc +++ b/storage/spider/ha_spider.cc @@ -4882,10 +4882,6 @@ int ha_spider::rnd_init( } } pushed_pos = NULL; -/* - if (wide_handler->external_lock_type == F_WRLCK) - check_and_start_bulk_update(SPD_BU_START_BY_INDEX_OR_RND_INIT); -*/ rnd_scan_and_first = scan; if ( scan && diff --git a/storage/spider/mysql-test/spider/bg/t/basic_sql.test b/storage/spider/mysql-test/spider/bg/t/basic_sql.test index 6421198fa95..3aae2e34455 100644 --- a/storage/spider/mysql-test/spider/bg/t/basic_sql.test +++ b/storage/spider/mysql-test/spider/bg/t/basic_sql.test @@ -1039,8 +1039,11 @@ if ($USE_CHILD_GROUP2) } } --connection master_1 +# MDEV-36357 +--disable_view_protocol SELECT a.a, a.b, date_format(a.c, '%Y-%m-%d %H:%i:%s') FROM tb_l a WHERE EXISTS (SELECT * FROM ta_l b WHERE b.b = a.b) ORDER BY a.a; +--enable_view_protocol if ($USE_CHILD_GROUP2) { if (!$OUTPUT_CHILD_GROUP2) diff --git a/storage/spider/spd_db_conn.cc b/storage/spider/spd_db_conn.cc index d33115ca1a0..46d9757cefb 100644 --- a/storage/spider/spd_db_conn.cc +++ b/storage/spider/spd_db_conn.cc @@ -3553,6 +3553,10 @@ int spider_db_store_result( db_conn = conn->db_conn; if (!result_list->current) { + /* + Point ->current and ->bgs_current to ->first (create ->first + if needed) + */ if (!result_list->first) { if (!(result_list->first = (SPIDER_RESULT *) @@ -3577,13 +3581,17 @@ int spider_db_store_result( } result_list->bgs_current = result_list->current; current = (SPIDER_RESULT*) result_list->current; - } else { + } else { /* result_list->current != NULL */ if ( #ifndef WITHOUT_SPIDER_BG_SEARCH result_list->bgs_phase > 0 || #endif result_list->quick_phase > 0 ) { + /* + Advance bgs_current to the next result. Create a new result + if needed + */ if (result_list->bgs_current == result_list->last) { if (!(result_list->last = (SPIDER_RESULT *) @@ -3628,6 +3636,10 @@ int spider_db_store_result( } current = (SPIDER_RESULT*) result_list->bgs_current; } else { + /* + Advance current to the next result. Create a new result if + needed + */ if (result_list->current == result_list->last) { if (!(result_list->last = (SPIDER_RESULT *) diff --git a/storage/spider/spd_db_include.h b/storage/spider/spd_db_include.h index bcb16ab391b..26909be1d99 100644 --- a/storage/spider/spd_db_include.h +++ b/storage/spider/spd_db_include.h @@ -1731,7 +1731,7 @@ typedef struct st_spider_result st_spider_result *next; SPIDER_POSITION *first_position; /* for quick mode */ int pos_page_size; /* for quick mode */ - longlong record_num; + longlong record_num; /* number of rows */ bool finish_flg; bool use_position; bool first_pos_use_position; @@ -1783,7 +1783,7 @@ typedef struct st_spider_result_list bool sorted; bool desc_flg; longlong current_row_num; - longlong record_num; + longlong record_num; /* number of rows */ bool finish_flg; longlong limit_num; longlong internal_offset; From f99586668a79dd571a7dee7bce916d6b894d072a Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Fri, 11 Apr 2025 08:28:42 +0200 Subject: [PATCH 3/8] MDEV-36380 User has unauthorized access to a sequence through a view with security invoker check sequence privileges in Item_func_nextval::fix_fields(), just like column privileges are checked in Item_field::fix_fields() remove sequence specific hacks that kinda made sequence privilege checks works, but not in all cases. And they were too lax, didn't requre SELECT privilege for NEXTVAL. Also INSERT privilege looks wrong here, UPDATE would've been more appropriate, but won't change that for compatibility reasons. also fixes MDEV-36413 User without any privileges to a sequence can read from it and modify it via column default --- mysql-test/main/view_grant.result | 46 +++++++++++++++++ mysql-test/main/view_grant.test | 47 +++++++++++++++++ mysql-test/suite/sql_sequence/grant.result | 47 ++++++++++++++++- mysql-test/suite/sql_sequence/grant.test | 50 ++++++++++++++++++- mysql-test/suite/sql_sequence/gtid.result | 2 +- .../suite/sql_sequence/replication.result | 2 +- mysql-test/suite/sql_sequence/view.test | 1 - sql/item_func.cc | 10 ++++ sql/item_func.h | 7 +++ sql/sql_acl.cc | 14 ++---- sql/sql_parse.cc | 11 +--- 11 files changed, 211 insertions(+), 26 deletions(-) diff --git a/mysql-test/main/view_grant.result b/mysql-test/main/view_grant.result index f1bebf98ecd..57c0a0fcc55 100644 --- a/mysql-test/main/view_grant.result +++ b/mysql-test/main/view_grant.result @@ -1985,4 +1985,50 @@ connection default; DROP VIEW v1; DROP USER foo; DROP USER FOO; +# +# MDEV-36380: User has unauthorized access to a sequence through +# a view with security invoker +# +create database db; +use db; +create sequence s; +create sql security invoker view vin as select nextval(s); +create sql security definer view vdn as select nextval(s); +create sql security invoker view vil as select lastval(s); +create sql security definer view vdl as select lastval(s); +create sql security invoker view vis as select setval(s,20); +create sql security definer view vds as select setval(s,30); +create user u@localhost; +grant select on db.vin to u@localhost; +grant select on db.vdn to u@localhost; +grant select on db.vil to u@localhost; +grant select on db.vdl to u@localhost; +grant select on db.vis to u@localhost; +grant select on db.vds to u@localhost; +connect con1,localhost,u,,db; +select nextval(s); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vin; +ERROR HY000: View 'db.vin' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vdn; +nextval(s) +1 +select lastval(s); +ERROR 42000: SELECT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vil; +ERROR HY000: View 'db.vil' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vdl; +lastval(s) +1 +select setval(s,10); +ERROR 42000: INSERT command denied to user 'u'@'localhost' for table `db`.`s` +select * from vis; +ERROR HY000: View 'db.vis' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them +select * from vds; +setval(s,30) +30 +disconnect con1; +connection default; +drop database db; +drop user u@localhost; # End of 10.5 tests diff --git a/mysql-test/main/view_grant.test b/mysql-test/main/view_grant.test index fc0521933cc..d61362b6cc3 100644 --- a/mysql-test/main/view_grant.test +++ b/mysql-test/main/view_grant.test @@ -2239,4 +2239,51 @@ DROP VIEW v1; DROP USER foo; DROP USER FOO; +--echo # +--echo # MDEV-36380: User has unauthorized access to a sequence through +--echo # a view with security invoker +--echo # +create database db; +use db; +create sequence s; +create sql security invoker view vin as select nextval(s); +create sql security definer view vdn as select nextval(s); +create sql security invoker view vil as select lastval(s); +create sql security definer view vdl as select lastval(s); +create sql security invoker view vis as select setval(s,20); +create sql security definer view vds as select setval(s,30); +create user u@localhost; +grant select on db.vin to u@localhost; +grant select on db.vdn to u@localhost; +grant select on db.vil to u@localhost; +grant select on db.vdl to u@localhost; +grant select on db.vis to u@localhost; +grant select on db.vds to u@localhost; + +--connect (con1,localhost,u,,db) +--error ER_TABLEACCESS_DENIED_ERROR +select nextval(s); +--error ER_VIEW_INVALID +select * from vin; +--disable_ps2_protocol +select * from vdn; +--enable_ps2_protocol + +--error ER_TABLEACCESS_DENIED_ERROR +select lastval(s); +--error ER_VIEW_INVALID +select * from vil; +select * from vdl; + +--error ER_TABLEACCESS_DENIED_ERROR +select setval(s,10); +--error ER_VIEW_INVALID +select * from vis; +select * from vds; + +--disconnect con1 +--connection default +drop database db; +drop user u@localhost; + --echo # End of 10.5 tests diff --git a/mysql-test/suite/sql_sequence/grant.result b/mysql-test/suite/sql_sequence/grant.result index 0a69d69fc74..fc3421efcb6 100644 --- a/mysql-test/suite/sql_sequence/grant.result +++ b/mysql-test/suite/sql_sequence/grant.result @@ -47,14 +47,57 @@ next_not_cached_value minimum_value maximum_value start_value increment cache_si 11 1 9223372036854775806 1 1 1000 0 0 connection only_alter; select next value for s1; -ERROR 42000: INSERT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` alter sequence s1 restart= 11; select * from s1; ERROR 42000: SELECT command denied to user 'only_alter'@'localhost' for table `mysqltest_1`.`s1` connection default; -drop database mysqltest_1; drop user 'normal'@'%'; drop user 'read_only'@'%'; drop user 'read_write'@'%'; drop user 'alter'@'%'; drop user 'only_alter'@'%'; +drop sequence s1; +# +# MDEV-36413 User without any privileges to a sequence can read from +# it and modify it via column default +# +create sequence s1; +create sequence s2; +select * from s2; +next_not_cached_value minimum_value maximum_value start_value increment cache_size cycle_option cycle_count +1 1 9223372036854775806 1 1 1000 0 0 +create table t2 (a int not null default(nextval(s1))); +insert into t2 values(); +create user u; +grant create, insert, select, drop on mysqltest_1.t1 to u; +grant insert, select on mysqltest_1.s1 to u; +grant select on mysqltest_1.t2 to u; +connect con1,localhost,u,,mysqltest_1; +select nextval(s2); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +show create sequence s2; +ERROR 42000: SHOW command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +create table t1 (a int not null default(nextval(s1))); +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from t2; +insert into t1 values(); +select * from t1; +a +1 +2 +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from (select t2.a from t2,t2 as t3 where t2.a=t3.a) as t4; +drop table t1; +create table t1 (a int not null default(nextval(s2))); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +create table t1 (a int not null default(nextval(s1)), +b int not null default(nextval(s2))); +ERROR 42000: SELECT, INSERT command denied to user 'u'@'localhost' for table `mysqltest_1`.`s2` +disconnect con1; +connection default; +drop user u; +drop database mysqltest_1; +# +# End of 10.11 tests +# diff --git a/mysql-test/suite/sql_sequence/grant.test b/mysql-test/suite/sql_sequence/grant.test index fb8a9f813a6..c205bd34223 100644 --- a/mysql-test/suite/sql_sequence/grant.test +++ b/mysql-test/suite/sql_sequence/grant.test @@ -60,10 +60,58 @@ select * from s1; # connection default; -drop database mysqltest_1; drop user 'normal'@'%'; drop user 'read_only'@'%'; drop user 'read_write'@'%'; drop user 'alter'@'%'; drop user 'only_alter'@'%'; +drop sequence s1; +--echo # +--echo # MDEV-36413 User without any privileges to a sequence can read from +--echo # it and modify it via column default +--echo # + +create sequence s1; +create sequence s2; +select * from s2; +create table t2 (a int not null default(nextval(s1))); +insert into t2 values(); + +create user u; +grant create, insert, select, drop on mysqltest_1.t1 to u; +grant insert, select on mysqltest_1.s1 to u; +grant select on mysqltest_1.t2 to u; + +--connect(con1,localhost,u,,mysqltest_1) +--error ER_TABLEACCESS_DENIED_ERROR +select nextval(s2); +--error ER_TABLEACCESS_DENIED_ERROR +show create sequence s2; + +create table t1 (a int not null default(nextval(s1))); +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from t2; +insert into t1 values(); +select * from t1; +drop table t1; +create table t1 (a int not null default(nextval(s1))) select a from (select t2.a from t2,t2 as t3 where t2.a=t3.a) as t4; +drop table t1; +--error ER_TABLEACCESS_DENIED_ERROR +create table t1 (a int not null default(nextval(s2))); +--error ER_TABLEACCESS_DENIED_ERROR +create table t1 (a int not null default(nextval(s1)), + b int not null default(nextval(s2))); +--disconnect con1 +--connection default +drop user u; + +# +# Cleanup +# + +drop database mysqltest_1; + +--echo # +--echo # End of 10.11 tests +--echo # diff --git a/mysql-test/suite/sql_sequence/gtid.result b/mysql-test/suite/sql_sequence/gtid.result index 5c0003d4ea3..f59dc5595ee 100644 --- a/mysql-test/suite/sql_sequence/gtid.result +++ b/mysql-test/suite/sql_sequence/gtid.result @@ -174,7 +174,7 @@ create sequence s_db.s2; drop sequence s_db.s2; connection m_normal_2; select next value for s_db.s1; -ERROR 42000: INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` create sequence s_db.s2; ERROR 42000: CREATE command denied to user 'normal_2'@'localhost' for table `s_db`.`s2` connection m_normal_1; diff --git a/mysql-test/suite/sql_sequence/replication.result b/mysql-test/suite/sql_sequence/replication.result index 762c332dbd6..0e4ca1c939e 100644 --- a/mysql-test/suite/sql_sequence/replication.result +++ b/mysql-test/suite/sql_sequence/replication.result @@ -285,7 +285,7 @@ create sequence s_db.s2; drop sequence s_db.s2; connection m_normal_2; select NEXT VALUE for s_db.s1; -ERROR 42000: INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` +ERROR 42000: SELECT, INSERT command denied to user 'normal_2'@'localhost' for table `s_db`.`s1` create sequence s_db.s2; ERROR 42000: CREATE command denied to user 'normal_2'@'localhost' for table `s_db`.`s2` connection m_normal_1; diff --git a/mysql-test/suite/sql_sequence/view.test b/mysql-test/suite/sql_sequence/view.test index affac002878..7e2cd712325 100644 --- a/mysql-test/suite/sql_sequence/view.test +++ b/mysql-test/suite/sql_sequence/view.test @@ -1,5 +1,4 @@ --source include/have_sequence.inc ---source include/have_innodb.inc # # Test sequences with views diff --git a/sql/item_func.cc b/sql/item_func.cc index 6a7b12ae4a0..3ca3d5af3a3 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7092,6 +7092,16 @@ longlong Item_func_cursor_rowcount::val_int() /***************************************************************************** SEQUENCE functions *****************************************************************************/ +bool Item_func_nextval::check_access_and_fix_fields(THD *thd, Item **ref, + privilege_t want_access) +{ + table_list->sequence= false; + bool error= check_single_table_access(thd, want_access, table_list, false); + table_list->sequence= true; + if (error && table_list->belong_to_view) + table_list->hide_view_error(thd); + return error || Item_longlong_func::fix_fields(thd, ref); +} longlong Item_func_nextval::val_int() { diff --git a/sql/item_func.h b/sql/item_func.h index f4f0558a77b..68b3669c3b4 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -3774,11 +3774,14 @@ class Item_func_nextval :public Item_longlong_func protected: TABLE_LIST *table_list; TABLE *table; + bool check_access_and_fix_fields(THD *, Item **ref, privilege_t); public: Item_func_nextval(THD *thd, TABLE_LIST *table_list_arg): Item_longlong_func(thd), table_list(table_list_arg) {} longlong val_int() override; const char *func_name() const override { return "nextval"; } + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, INSERT_ACL | SELECT_ACL); } bool fix_length_and_dec() override { unsigned_flag= 0; @@ -3820,6 +3823,8 @@ class Item_func_lastval :public Item_func_nextval public: Item_func_lastval(THD *thd, TABLE_LIST *table_list_arg): Item_func_nextval(thd, table_list_arg) {} + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, SELECT_ACL); } longlong val_int() override; const char *func_name() const override { return "lastval"; } Item *do_get_copy(THD *thd) const override @@ -3840,6 +3845,8 @@ public: : Item_func_nextval(thd, table_list_arg), nextval(nextval_arg), round(round_arg), is_used(is_used_arg) {} + bool fix_fields(THD *thd, Item **ref) override + { return check_access_and_fix_fields(thd, ref, INSERT_ACL); } longlong val_int() override; const char *func_name() const override { return "setval"; } void print(String *str, enum_query_type query_type) override; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 385d3da18e2..939061fdf8e 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -8304,19 +8304,13 @@ bool check_grant(THD *thd, privilege_t want_access, TABLE_LIST *tables, /* If sequence is used as part of NEXT VALUE, PREVIOUS VALUE or SELECT, - we need to modify the requested access rights depending on how the - sequence is used. + the privilege will be checked in ::fix_fields(). + Direct SELECT of a sequence table doesn't set t_ref->sequence, so + privileges will be checked normally, as for any table. */ if (t_ref->sequence && !(want_access & ~(SELECT_ACL | INSERT_ACL | UPDATE_ACL | DELETE_ACL))) - { - /* - We want to have either SELECT or INSERT rights to sequences depending - on how they are accessed - */ - orig_want_access= ((t_ref->lock_type == TL_WRITE_ALLOW_WRITE) ? - INSERT_ACL : SELECT_ACL); - } + continue; const ACL_internal_table_access *access= get_cached_table_access(&t_ref->grant.m_internal, diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 705dc35cdb7..b017cba3c36 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7332,18 +7332,9 @@ check_table_access(THD *thd, privilege_t requirements, TABLE_LIST *tables, DBUG_PRINT("info", ("derived: %d view: %d", table_ref->derived != 0, table_ref->view != 0)); - if (table_ref->is_anonymous_derived_table()) + if (table_ref->is_anonymous_derived_table() || table_ref->sequence) continue; - if (table_ref->sequence) - { - /* We want to have either SELECT or INSERT rights to sequences depending - on how they are accessed - */ - want_access= ((table_ref->lock_type == TL_WRITE_ALLOW_WRITE) ? - INSERT_ACL : SELECT_ACL); - } - if (check_access(thd, want_access, table_ref->get_db_name().str, &table_ref->grant.privilege, &table_ref->grant.m_internal, From 7f1492d0bc01fe24a55a3c5f9a9314b5c91d79d6 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 17 Apr 2025 17:08:04 +0200 Subject: [PATCH 4/8] cleanup: rename hide_view_error->replace_view_error_with_generic as requested by Monty --- sql/item.cc | 6 +++--- sql/item_func.cc | 2 +- sql/sql_base.cc | 2 +- sql/sql_update.cc | 2 +- sql/sql_view.cc | 2 +- sql/table.cc | 4 ++-- sql/table.h | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sql/item.cc b/sql/item.cc index 9498d816bb0..466ba63b1ce 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -10930,8 +10930,8 @@ void dummy_error_processor(THD *thd, void *data) {} /** - Wrapper of hide_view_error call for Name_resolution_context error - processor. + Wrapper of replace_view_error_with_generic call for Name_resolution_context + error processor. @note hide view underlying tables details in error messages @@ -10939,7 +10939,7 @@ void dummy_error_processor(THD *thd, void *data) void view_error_processor(THD *thd, void *data) { - ((TABLE_LIST *)data)->hide_view_error(thd); + ((TABLE_LIST *)data)->replace_view_error_with_generic(thd); } /** diff --git a/sql/item_func.cc b/sql/item_func.cc index 3ca3d5af3a3..5bea3f440b4 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7099,7 +7099,7 @@ bool Item_func_nextval::check_access_and_fix_fields(THD *thd, Item **ref, bool error= check_single_table_access(thd, want_access, table_list, false); table_list->sequence= true; if (error && table_list->belong_to_view) - table_list->hide_view_error(thd); + table_list->replace_view_error_with_generic(thd); return error || Item_longlong_func::fix_fields(thd, ref); } diff --git a/sql/sql_base.cc b/sql/sql_base.cc index ac4bce1dce0..ed23208e358 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8009,7 +8009,7 @@ bool setup_tables_and_check_access(THD *thd, Name_resolution_context *context, if (table_list->belong_to_view && !table_list->view && check_single_table_access(thd, access, table_list, FALSE)) { - tables->hide_view_error(thd); + tables->replace_view_error_with_generic(thd); DBUG_RETURN(TRUE); } access= want_access; diff --git a/sql/sql_update.cc b/sql/sql_update.cc index f2cea510ce7..1327bd786c6 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -1684,7 +1684,7 @@ static bool multi_update_check_table_access(THD *thd, TABLE_LIST *table, if (multi_update_check_table_access(thd, tbl, tables_for_update, &updated)) { - tbl->hide_view_error(thd); + tbl->replace_view_error_with_generic(thd); return true; } } diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 89d0704f6df..9df5c89cf37 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -337,7 +337,7 @@ bool create_view_precheck(THD *thd, TABLE_LIST *tables, TABLE_LIST *view, { if (check_single_table_access(thd, SELECT_ACL, tbl, FALSE)) { - tbl->hide_view_error(thd); + tbl->replace_view_error_with_generic(thd); goto err; } } diff --git a/sql/table.cc b/sql/table.cc index 978dbee254e..1b5d6741b45 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -6065,9 +6065,9 @@ bool TABLE_LIST::prep_check_option(THD *thd, uint8 check_opt_type) @pre This method can be called only if there is an error. */ -void TABLE_LIST::hide_view_error(THD *thd) +void TABLE_LIST::replace_view_error_with_generic(THD *thd) { - if ((thd->killed && !thd->is_error())|| thd->get_internal_handler()) + if ((thd->killed && !thd->is_error()) || thd->get_internal_handler()) return; /* Hide "Unknown column" or "Unknown function" error */ DBUG_ASSERT(thd->is_error()); diff --git a/sql/table.h b/sql/table.h index 981fc5d23c5..eeec8b05022 100644 --- a/sql/table.h +++ b/sql/table.h @@ -2776,7 +2776,7 @@ struct TABLE_LIST bool check_single_table(TABLE_LIST **table, table_map map, TABLE_LIST *view); bool set_insert_values(MEM_ROOT *mem_root); - void hide_view_error(THD *thd); + void replace_view_error_with_generic(THD *thd); TABLE_LIST *find_underlying_table(TABLE *table); TABLE_LIST *first_leaf_for_name_resolution(); TABLE_LIST *last_leaf_for_name_resolution(); From 8c6b0d092ad2215ef0a1ac17f0ac7e3d087556d3 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Thu, 6 Mar 2025 05:27:48 +0000 Subject: [PATCH 5/8] MDEV-36245 Long server_audit_file_path causes buffer overflow Limit size of server_audit_file_path value Currently, the length of this value is not checked and can cause a buffer overflow if given a long file path specifying a directory. In file_logger:logger_open(), there is a check: ``` if (new_log.path_len+n_dig(rotations)+1 > FN_REFLEN) // handle error ``` As n_dig(rotations) may return up to 3, this inherently limits the file path to FN_REFLEN - 4 characters. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc. --- plugin/server_audit/server_audit.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c index 8b49f354f47..183295773f5 100644 --- a/plugin/server_audit/server_audit.c +++ b/plugin/server_audit/server_audit.c @@ -2836,6 +2836,14 @@ static void update_file_path(MYSQL_THD thd, { char *new_name= (*(char **) save) ? *(char **) save : empty_str; + if (strlen(new_name) + 4 > FN_REFLEN) + { + error_header(); + fprintf(stderr, "server_audit_file_path can't exceed %d characters.\n", FN_REFLEN - 4); + CLIENT_ERROR(1, "server_audit_file_path can't exceed %d characters.\n", MYF(ME_WARNING), FN_REFLEN - 4); + return; + } + ADD_ATOMIC(internal_stop_logging, 1); error_header(); fprintf(stderr, "Log file name was changed to '%s'.\n", new_name); From fbec528cbb577744a4341602926113c3fe48c080 Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Thu, 6 Mar 2025 05:27:48 +0000 Subject: [PATCH 6/8] MDEV-36245 review changes Closes #3874 --- mysql-test/suite/plugins/r/server_audit.result | 3 +++ mysql-test/suite/plugins/t/server_audit.test | 4 ++++ plugin/server_audit/server_audit.c | 8 ++++++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/plugins/r/server_audit.result b/mysql-test/suite/plugins/r/server_audit.result index fc9d98f5d0a..06b60448942 100644 --- a/mysql-test/suite/plugins/r/server_audit.result +++ b/mysql-test/suite/plugins/r/server_audit.result @@ -20,6 +20,9 @@ set global server_audit_file_path=null; set global server_audit_incl_users=null; set global server_audit_file_path='server_audit.log'; set global server_audit_output_type=file; +set global server_audit_file_path=REPEAT(REPEAT('new_file_name', 50), 50); +Warnings: +Warning 1 server_audit_file_path can't exceed FN_LEN characters. set global server_audit_logging=on; set global server_audit_incl_users= repeat("'root',", 10000); ERROR 42000: Variable 'server_audit_incl_users' can't be set to the value of ''root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','root','...' diff --git a/mysql-test/suite/plugins/t/server_audit.test b/mysql-test/suite/plugins/t/server_audit.test index 47d6fdacdcf..673bc7ff221 100644 --- a/mysql-test/suite/plugins/t/server_audit.test +++ b/mysql-test/suite/plugins/t/server_audit.test @@ -20,6 +20,10 @@ set global server_audit_file_path=null; set global server_audit_incl_users=null; set global server_audit_file_path='server_audit.log'; set global server_audit_output_type=file; + +--replace_regex /[1-9][0-9][0-9]+/FN_LEN/ +set global server_audit_file_path=REPEAT(REPEAT('new_file_name', 50), 50); + set global server_audit_logging=on; --error ER_WRONG_VALUE_FOR_VAR diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c index 183295773f5..5c2c65c4161 100644 --- a/plugin/server_audit/server_audit.c +++ b/plugin/server_audit/server_audit.c @@ -2839,8 +2839,12 @@ static void update_file_path(MYSQL_THD thd, if (strlen(new_name) + 4 > FN_REFLEN) { error_header(); - fprintf(stderr, "server_audit_file_path can't exceed %d characters.\n", FN_REFLEN - 4); - CLIENT_ERROR(1, "server_audit_file_path can't exceed %d characters.\n", MYF(ME_WARNING), FN_REFLEN - 4); + fprintf(stderr, + "server_audit_file_path can't exceed %d characters.\n", + FN_REFLEN - 4); + fprintf(stderr, "Log filename remains unchanged '%s'.\n", file_path); + CLIENT_ERROR(1, "server_audit_file_path can't exceed %d characters.", + MYF(ME_WARNING), FN_REFLEN - 4); return; } From 5f6b92a738a4380a46494206bf1ceec2999053c5 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Sun, 20 Apr 2025 10:01:52 +0200 Subject: [PATCH 7/8] New CC --- libmariadb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmariadb b/libmariadb index 7d930974c0c..867f0d18253 160000 --- a/libmariadb +++ b/libmariadb @@ -1 +1 @@ -Subproject commit 7d930974c0c5588a8872f30b83d6bc429106f825 +Subproject commit 867f0d18253f48eb5bead9e6636ec06824017aeb From 952ffb55f93bf6a6c5d1c9617e5a2a56207ee674 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Sun, 20 Apr 2025 19:01:49 +0200 Subject: [PATCH 8/8] Fix valgrind detection --- mysql-test/main/sp-no-valgrind.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/main/sp-no-valgrind.test b/mysql-test/main/sp-no-valgrind.test index 10a238f9871..72ef9fb36a9 100644 --- a/mysql-test/main/sp-no-valgrind.test +++ b/mysql-test/main/sp-no-valgrind.test @@ -1,5 +1,5 @@ --source include/not_msan.inc ---source include/not_valgrind_build.inc +--source include/not_valgrind.inc --echo # MDEV-20699 do not cache SP in SHOW CREATE --echo # Warmup round, this might allocate some memory for session variable