Hi, Monty, On Apr 01, Michael Widenius wrote:

revision-id: 0a0c5f349c6 (mariadb-11.8.1-42-g0a0c5f349c6) parent(s): 21321025e5e author: Michael Widenius committer: Michael Widenius timestamp: 2025-03-31 20:09:03 +0300 message:

MDEV-36425 Extend read_only to also block share locks and super user

The main purpose of this allow one to use the --read-only option to ensure that no one can issue a query that can block replication.

The --read-only option can now take 4 different values: 0 No read only (as before). 1 Blocks changes for users without the 'READ ONLY ADMIN' privilege (as before). 2 Blocks in addition LOCK TABLES and SELECT IN SHARE MODE for not 'READ ONLY ADMIN' users. 3 Blocks in addition 'READ_ONLY_ADMIN' users for all the previous statements.

diff --git a/mysql-test/main/read_only.result b/mysql-test/main/read_only.result index 65cc12ffce9..89390ad65d5 100644 --- a/mysql-test/main/read_only.result +++ b/mysql-test/main/read_only.result @@ -69,11 +69,14 @@ set global read_only=1; connection con1; select @@global.read_only; @@global.read_only -0 +OFF unlock tables ; +Timeout in wait_condition.inc for SELECT @@global.read_only= 1 +Id User Host db Command Time State Info Progress +5 test localhost test Query 0 starting show full processlist 0.000

Forgot to fix? It should wait for @@global.read_only='READ_ONLY' now

select @@global.read_only; @@global.read_only -1 +READ_ONLY connection default; reap; connection default; diff --git a/sql/lock.cc b/sql/lock.cc index 1296e5455d5..8f9f97f88b7 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -179,19 +198,48 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)

/* Prevent modifications to base tables if READ_ONLY is activated. - In any case, read only does not apply to temporary tables. + In any case, read only does not apply to temporary tables or slave + threads. */ - if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table) + if (unlikely(opt_readonly) && + !(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table && + !thd->slave_thread) { - if (t->reginfo.lock_type >= TL_FIRST_WRITE && - !ignore_read_only && opt_readonly && !thd->slave_thread) + switch (opt_readonly) // Impossible

you have "Impossible" comment on a wrong line

{ - my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); - DBUG_RETURN(1); + case READONLY_OFF: + DBUG_ASSERT(0); + break; + case READONLY_ON: // Compatibility + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && + t->reginfo.lock_type >= TL_FIRST_WRITE) + { + mariadb_error_read_only(); + DBUG_RETURN(1);

I'd make mariadb_error_read_only() to return 1. Then you can write DBUG_RETURN(mariadb_error_read_only()); here and also in all other places, check it out, it's always followed by return 1.

+ } + break; + case READONLY_NO_LOCK: + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && + (thd->lex->sql_command == SQLCOM_LOCK_TABLES || + t->reginfo.lock_type >= TL_FIRST_WRITE || + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS))

may be t->reginfo.lock_type >= TL_READ_WITH_SHARED_LOCKS ?

+ { + mariadb_error_read_only(); + DBUG_RETURN(1); + } + break; + case READONLY_NO_LOCK_NO_ADMIN: + if (thd->lex->sql_command == SQLCOM_LOCK_TABLES || + t->reginfo.lock_type >= TL_FIRST_WRITE || + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS)

and here

+ { + mariadb_error_read_only(); + DBUG_RETURN(1); + } + break; } } } - /* Locking of system tables is restricted: locking a mix of system and non-system tables in the same lock diff --git a/sql/sql_class.h b/sql/sql_class.h index d65cec37154..34ee8391009 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3448,9 +3452,12 @@ class THD: public THD_count, /* this must be first */ */ inline bool is_read_only_ctx()

rename to, for example, check_read_only_with_errror()

{ - return opt_readonly && - !(security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && - !slave_thread; + if (likely(!opt_readonly) || slave_thread || + ((security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && + opt_readonly != READONLY_NO_LOCK_NO_ADMIN)) + return false; + mariadb_error_read_only(); + return true; }

private: diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 0247afd91a6..545b4674da1 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3253,15 +3253,24 @@ static bool fix_read_only(sys_var *self, THD *thd, enum_var_type type) transition (especially when transitioning from false to true) and synchronizes both booleans in the end. */ -static Sys_var_on_access_global<Sys_var_mybool, + +const char *read_only_mode_names[]= +{"OFF", "READ_ONLY", "READ_ONLY_NO_LOCK", "READ_ONLY_NO_LOCK_NO_ADMIN"}; + +static Sys_var_on_access_global<Sys_var_enum, PRIV_SET_SYSTEM_GLOBAL_VAR_READ_ONLY> Sys_readonly( "read_only", - "Make all non-temporary tables read-only, with the exception for " - "replication (slave) threads and users with the 'READ ONLY ADMIN' " - "privilege", - GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), DEFAULT(FALSE), - NO_MUTEX_GUARD, NOT_IN_BINLOG, + "Do not allow changes to non-temporary tables. Options are:" + "OFF changes allowed. " + "READ_ONLY blocks changes for users without the 'READ ONLY ADMIN' "

"Disallow changes for users without the READ ONLY ADMIN privilege"

+ "privilege. " + "READ_ONLY_NO_LOCK blocks in addition LOCK TABLES and " + "SELECT IN SHARE MODE for not ADMIN users. "

"Additionally disallows LOCK TABLES and SELECT IN SHARE MODE"

+ "READ_ONLY_NO_LOCK_NO_ADMIN blocks in addition 'ADMIN' users. "

"Disallows also for users with READ_ONLY ADMIN privilege"

+ "Replication (slave) threads are not affected by this option", + GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), + read_only_mode_names, DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_read_only), ON_UPDATE(fix_read_only));

// Small lower limit to be able to test MRR

Regards, Sergei Chief Architect, MariaDB Server and security@mariadb.org