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(a)mariadb.org
