Skip to content

Commit

Permalink
MENT-1707 Crash at reload_acl_and_cache
Browse files Browse the repository at this point in the history
The stack function trace for this bug is:

libc
my_free
free_root
acl_reload

The crash happens because acl_memroot gets corrupted.

The issue was that during FLUSH PRIVILEGES we discard the old
privileges and create new ones. We have protection in place that no
one can accesses the privileges during this time.

However one short piece of code called during login of a new user, or
change password, was not properly protected, which could in some very
rare circumstances case a memory overwrite of a MEMROOT object if
at the same time another thread calls FLUSH PRIVILEGES.

This it issue is fixed by adding protection around set_user_salt().
I also added asserts to other code that is using the acl_memroot to
ensure that it is properly proteced everywhere.
  • Loading branch information
montywi committed Nov 27, 2023
1 parent 18acf97 commit 9e424b6
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion sql/sql_acl.cc
Expand Up @@ -934,6 +934,7 @@ class User_table_tabular: public User_table

int get_auth(THD *thd, MEM_ROOT *root, ACL_USER *u) const
{
mysql_mutex_assert_owner(&acl_cache->lock);
u->alloc_auth(root, 1);
if (have_password())
{
Expand Down Expand Up @@ -2144,6 +2145,9 @@ static bool validate_password(THD *thd, const LEX_CSTRING &user,
static int set_user_salt(ACL_USER::AUTH *auth, plugin_ref plugin)
{
st_mysql_auth *info= (st_mysql_auth *) plugin_decl(plugin)->info;

mysql_mutex_assert_owner(&acl_cache->lock);

if (info->interface_version >= 0x0202 && info->preprocess_hash &&
auth->auth_string.length)
{
Expand Down Expand Up @@ -2178,6 +2182,8 @@ static int set_user_auth(THD *thd, const LEX_CSTRING &user,
plugin_ref plugin= get_auth_plugin(thd, auth->plugin, &unlock_plugin);
int res= 1;

mysql_mutex_assert_owner(&acl_cache->lock);

if (!plugin)
{
push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
Expand Down Expand Up @@ -2254,10 +2260,13 @@ static bool set_user_salt_if_needed(ACL_USER *user_copy, int curr_auth,
if (auth_copy->salt.str)
return 0; // already done

mysql_mutex_lock(&acl_cache->lock);
if (set_user_salt(auth_copy, plugin))
{
mysql_mutex_unlock(&acl_cache->lock);
return 1;
}

mysql_mutex_lock(&acl_cache->lock);
ACL_USER *user= find_user_exact(user_copy->host.hostname, user_copy->user.str);
// make sure the user wasn't altered or dropped meanwhile
if (user)
Expand Down Expand Up @@ -3280,6 +3289,7 @@ ACL_USER::ACL_USER(THD *thd, const LEX_USER &combo,
const Account_options &options,
const ulong privileges)
{
mysql_mutex_assert_owner(&acl_cache->lock);
user= safe_lexcstrdup_root(&acl_memroot, combo.user);
update_hostname(&host, safe_strdup_root(&acl_memroot, combo.host.str));
hostname_length= combo.host.length;
Expand All @@ -3296,6 +3306,8 @@ static int acl_user_update(THD *thd, ACL_USER *acl_user, uint nauth,
const ulong privileges)
{
ACL_USER_PARAM::AUTH *work_copy= NULL;
mysql_mutex_assert_owner(&acl_cache->lock);

if (nauth)
{
if (!(work_copy= (ACL_USER_PARAM::AUTH*)
Expand Down Expand Up @@ -4971,6 +4983,7 @@ update_role_mapping(LEX_CSTRING *user, LEX_CSTRING *host, LEX_CSTRING *role,
return 0;
}

mysql_mutex_assert_owner(&acl_cache->lock);
/* allocate a new entry that will go in the hash */
ROLE_GRANT_PAIR *hash_entry= new (&acl_memroot) ROLE_GRANT_PAIR;
if (hash_entry->init(&acl_memroot, user->str, host->str,
Expand Down Expand Up @@ -5035,6 +5048,7 @@ replace_proxies_priv_table(THD *thd, TABLE *table, const LEX_USER *user,

DBUG_ENTER("replace_proxies_priv_table");

mysql_mutex_assert_owner(&acl_cache->lock);
if (!table)
{
my_error(ER_NO_SUCH_TABLE, MYF(0), MYSQL_SCHEMA_NAME.str,
Expand Down

0 comments on commit 9e424b6

Please sign in to comment.