Skip to content

M
MariaDB
Commit 3d45a067 authored by unknown's avatar unknown
Browse files
Options

Reapply fix for bug#16372 (Server crashes when test 'conc_sys' is running) 

after merge.

Concurrent read and update of privilege structures (like simultaneous
run of SHOW GRANTS and ADD USER) could result in server crash.

Ensure that proper locking of ACL structures is done.

No test case is provided because this bug can't be reproduced
deterministically.


sql/sql_acl.cc:
  Ensure that access to ACL data is protected by acl_cache->lock mutex.
  Use system_charset_info for host names consistently.
  Remove check_acl_user().  Use find_acl_user() instead.
sql/sql_acl.h:
  Remove check_acl_user() declaration.
sql/sql_parse.cc:
  Use is_acl_user() instead of check_acl_user().
parent 6a807244
Show whitespace changes
Inline Side-by-side
Showing with 63 additions and 50 deletions
sql/sql_acl.cc
View file @ 3d45a067
...@@ -1027,6 +1027,8 @@ static void acl_update_user(const char *user, const char *host, ...@@ -1027,6 +1027,8 @@ static void acl_update_user(const char *user, const char *host,
USER_RESOURCES *mqh, USER_RESOURCES *mqh,
ulong privileges) ulong privileges)
{ {
safe_mutex_assert_owner(&acl_cache->lock);
for (uint i=0 ; i < acl_users.elements ; i++) for (uint i=0 ; i < acl_users.elements ; i++)
{ {
ACL_USER *acl_user=dynamic_element(&acl_users,i,ACL_USER*); ACL_USER *acl_user=dynamic_element(&acl_users,i,ACL_USER*);
...@@ -1077,6 +1079,9 @@ static void acl_insert_user(const char *user, const char *host, ...@@ -1077,6 +1079,9 @@ static void acl_insert_user(const char *user, const char *host,
ulong privileges) ulong privileges)
{ {
ACL_USER acl_user; ACL_USER acl_user;
safe_mutex_assert_owner(&acl_cache->lock);
acl_user.user=*user ? strdup_root(&mem,user) : 0; acl_user.user=*user ? strdup_root(&mem,user) : 0;
update_hostname(&acl_user.host, *host ? strdup_root(&mem, host): 0); update_hostname(&acl_user.host, *host ? strdup_root(&mem, host): 0);
acl_user.access=privileges; acl_user.access=privileges;
...@@ -1106,6 +1111,8 @@ static void acl_insert_user(const char *user, const char *host, ...@@ -1106,6 +1111,8 @@ static void acl_insert_user(const char *user, const char *host,
static void acl_update_db(const char *user, const char *host, const char *db, static void acl_update_db(const char *user, const char *host, const char *db,
ulong privileges) ulong privileges)
{ {
safe_mutex_assert_owner(&acl_cache->lock);
for (uint i=0 ; i < acl_dbs.elements ; i++) for (uint i=0 ; i < acl_dbs.elements ; i++)
{ {
ACL_DB *acl_db=dynamic_element(&acl_dbs,i,ACL_DB*); ACL_DB *acl_db=dynamic_element(&acl_dbs,i,ACL_DB*);
...@@ -1532,6 +1539,9 @@ find_acl_user(const char *host, const char *user, my_bool exact) ...@@ -1532,6 +1539,9 @@ find_acl_user(const char *host, const char *user, my_bool exact)
{ {
DBUG_ENTER("find_acl_user"); DBUG_ENTER("find_acl_user");
DBUG_PRINT("enter",("host: '%s' user: '%s'",host,user)); DBUG_PRINT("enter",("host: '%s' user: '%s'",host,user));
safe_mutex_assert_owner(&acl_cache->lock);
for (uint i=0 ; i < acl_users.elements ; i++) for (uint i=0 ; i < acl_users.elements ; i++)
{ {
ACL_USER *acl_user=dynamic_element(&acl_users,i,ACL_USER*); ACL_USER *acl_user=dynamic_element(&acl_users,i,ACL_USER*);
...@@ -1544,7 +1554,7 @@ find_acl_user(const char *host, const char *user, my_bool exact) ...@@ -1544,7 +1554,7 @@ find_acl_user(const char *host, const char *user, my_bool exact)
if (!acl_user->user && !user[0] || if (!acl_user->user && !user[0] ||
acl_user->user && !strcmp(user,acl_user->user)) acl_user->user && !strcmp(user,acl_user->user))
{ {
if (exact ? !my_strcasecmp(&my_charset_latin1, host, if (exact ? !my_strcasecmp(system_charset_info, host,
acl_user->host.hostname ? acl_user->host.hostname ?
acl_user->host.hostname : "") : acl_user->host.hostname : "") :
compare_hostname(&acl_user->host,host,host)) compare_hostname(&acl_user->host,host,host))
...@@ -2868,6 +2878,7 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list, ...@@ -2868,6 +2878,7 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list,
create_new_users= test_if_create_new_users(thd); create_new_users= test_if_create_new_users(thd);
bool result= FALSE; bool result= FALSE;
rw_wrlock(&LOCK_grant); rw_wrlock(&LOCK_grant);
pthread_mutex_lock(&acl_cache->lock);
MEM_ROOT *old_root= thd->mem_root; MEM_ROOT *old_root= thd->mem_root;
thd->mem_root= &memex; thd->mem_root= &memex;
grant_version++; grant_version++;
...@@ -2885,12 +2896,10 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list, ...@@ -2885,12 +2896,10 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list,
continue; continue;
} }
/* Create user if needed */ /* Create user if needed */
pthread_mutex_lock(&acl_cache->lock);
error=replace_user_table(thd, tables[0].table, *Str, error=replace_user_table(thd, tables[0].table, *Str,
0, revoke_grant, create_new_users, 0, revoke_grant, create_new_users,
test(thd->variables.sql_mode & test(thd->variables.sql_mode &
MODE_NO_AUTO_CREATE_USER)); MODE_NO_AUTO_CREATE_USER));
pthread_mutex_unlock(&acl_cache->lock);
if (error) if (error)
{ {
result= TRUE; // Remember error result= TRUE; // Remember error
...@@ -2982,6 +2991,7 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list, ...@@ -2982,6 +2991,7 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table_list,
} }
grant_option=TRUE; grant_option=TRUE;
thd->mem_root= old_root; thd->mem_root= old_root;
pthread_mutex_unlock(&acl_cache->lock);
rw_unlock(&LOCK_grant); rw_unlock(&LOCK_grant);
if (!result) if (!result)
send_ok(thd); send_ok(thd);
...@@ -3074,6 +3084,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3074,6 +3084,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
if (!revoke_grant) if (!revoke_grant)
create_new_users= test_if_create_new_users(thd); create_new_users= test_if_create_new_users(thd);
rw_wrlock(&LOCK_grant); rw_wrlock(&LOCK_grant);
pthread_mutex_lock(&acl_cache->lock);
MEM_ROOT *old_root= thd->mem_root; MEM_ROOT *old_root= thd->mem_root;
thd->mem_root= &memex; thd->mem_root= &memex;
...@@ -3093,12 +3104,10 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3093,12 +3104,10 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
continue; continue;
} }
/* Create user if needed */ /* Create user if needed */
pthread_mutex_lock(&acl_cache->lock);
error=replace_user_table(thd, tables[0].table, *Str, error=replace_user_table(thd, tables[0].table, *Str,
0, revoke_grant, create_new_users, 0, revoke_grant, create_new_users,
test(thd->variables.sql_mode & test(thd->variables.sql_mode &
MODE_NO_AUTO_CREATE_USER)); MODE_NO_AUTO_CREATE_USER));
pthread_mutex_unlock(&acl_cache->lock);
if (error) if (error)
{ {
result= TRUE; // Remember error result= TRUE; // Remember error
...@@ -3140,6 +3149,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, ...@@ -3140,6 +3149,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc,
} }
grant_option=TRUE; grant_option=TRUE;
thd->mem_root= old_root; thd->mem_root= old_root;
pthread_mutex_unlock(&acl_cache->lock);
rw_unlock(&LOCK_grant); rw_unlock(&LOCK_grant);
if (!result && !no_error) if (!result && !no_error)
send_ok(thd); send_ok(thd);
...@@ -4116,20 +4126,15 @@ bool mysql_show_grants(THD *thd,LEX_USER *lex_user) ...@@ -4116,20 +4126,15 @@ bool mysql_show_grants(THD *thd,LEX_USER *lex_user)
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
} }
for (counter=0 ; counter < acl_users.elements ; counter++) rw_rdlock(&LOCK_grant);
{ VOID(pthread_mutex_lock(&acl_cache->lock));
const char *user,*host;
acl_user=dynamic_element(&acl_users,counter,ACL_USER*); acl_user= find_acl_user(lex_user->host.str, lex_user->user.str, TRUE);
if (!(user=acl_user->user)) if (!acl_user)
user= "";
if (!(host=acl_user->host.hostname))
host= "";
if (!strcmp(lex_user->user.str,user) &&
!my_strcasecmp(system_charset_info, lex_user->host.str, host))
break;
}
if (counter == acl_users.elements)
{ {
VOID(pthread_mutex_unlock(&acl_cache->lock));
rw_unlock(&LOCK_grant);
my_error(ER_NONEXISTING_GRANT, MYF(0), my_error(ER_NONEXISTING_GRANT, MYF(0),
lex_user->user.str, lex_user->host.str); lex_user->user.str, lex_user->host.str);
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
...@@ -4144,10 +4149,12 @@ bool mysql_show_grants(THD *thd,LEX_USER *lex_user) ...@@ -4144,10 +4149,12 @@ bool mysql_show_grants(THD *thd,LEX_USER *lex_user)
field_list.push_back(field); field_list.push_back(field);
if (protocol->send_fields(&field_list, if (protocol->send_fields(&field_list,
Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF))
DBUG_RETURN(TRUE); {
VOID(pthread_mutex_unlock(&acl_cache->lock));
rw_unlock(&LOCK_grant);
rw_wrlock(&LOCK_grant); DBUG_RETURN(TRUE);
VOID(pthread_mutex_lock(&acl_cache->lock)); }
/* Add first global access grants */ /* Add first global access grants */
{ {
...@@ -4555,10 +4562,15 @@ void get_privilege_desc(char *to, uint max_length, ulong access) ...@@ -4555,10 +4562,15 @@ void get_privilege_desc(char *to, uint max_length, ulong access)
void get_mqh(const char *user, const char *host, USER_CONN *uc) void get_mqh(const char *user, const char *host, USER_CONN *uc)
{ {
ACL_USER *acl_user; ACL_USER *acl_user;
pthread_mutex_lock(&acl_cache->lock);
if (initialized && (acl_user= find_acl_user(host,user, FALSE))) if (initialized && (acl_user= find_acl_user(host,user, FALSE)))
uc->user_resources= acl_user->user_resource; uc->user_resources= acl_user->user_resource;
else else
bzero((char*) &uc->user_resources, sizeof(uc->user_resources)); bzero((char*) &uc->user_resources, sizeof(uc->user_resources));
pthread_mutex_unlock(&acl_cache->lock);
} }
/* /*
...@@ -4638,31 +4650,6 @@ int open_grant_tables(THD *thd, TABLE_LIST *tables) ...@@ -4638,31 +4650,6 @@ int open_grant_tables(THD *thd, TABLE_LIST *tables)
DBUG_RETURN(0); DBUG_RETURN(0);
} }
ACL_USER *check_acl_user(LEX_USER *user_name,
uint *acl_acl_userdx)
{
ACL_USER *acl_user= 0;
uint counter;
for (counter= 0 ; counter < acl_users.elements ; counter++)
{
const char *user,*host;
acl_user= dynamic_element(&acl_users, counter, ACL_USER*);
if (!(user=acl_user->user))
user= "";
if (!(host=acl_user->host.hostname))
host= "";
if (!strcmp(user_name->user.str,user) &&
!my_strcasecmp(system_charset_info, user_name->host.str, host))
break;
}
if (counter == acl_users.elements)
return 0;
*acl_acl_userdx= counter;
return acl_user;
}
/* /*
Modify a privilege table. Modify a privilege table.
...@@ -4907,6 +4894,8 @@ static int handle_grant_struct(uint struct_no, bool drop, ...@@ -4907,6 +4894,8 @@ static int handle_grant_struct(uint struct_no, bool drop,
LINT_INIT(acl_db); LINT_INIT(acl_db);
LINT_INIT(grant_name); LINT_INIT(grant_name);
safe_mutex_assert_owner(&acl_cache->lock);
/* Get the number of elements in the in-memory structure. */ /* Get the number of elements in the in-memory structure. */
switch (struct_no) { switch (struct_no) {
case 0: case 0:
...@@ -5370,7 +5359,7 @@ bool mysql_revoke_all(THD *thd, List <LEX_USER> &list) ...@@ -5370,7 +5359,7 @@ bool mysql_revoke_all(THD *thd, List <LEX_USER> &list)
List_iterator <LEX_USER> user_list(list); List_iterator <LEX_USER> user_list(list);
while ((lex_user=user_list++)) while ((lex_user=user_list++))
{ {
if (!check_acl_user(lex_user, &counter)) if (!find_acl_user(lex_user->host.str, lex_user->user.str, TRUE))
{ {
sql_print_error("REVOKE ALL PRIVILEGES, GRANT: User '%s'@'%s' does not " sql_print_error("REVOKE ALL PRIVILEGES, GRANT: User '%s'@'%s' does not "
"exists", lex_user->user.str, lex_user->host.str); "exists", lex_user->user.str, lex_user->host.str);
...@@ -5606,6 +5595,7 @@ bool sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name, ...@@ -5606,6 +5595,7 @@ bool sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
combo->user.str= sctx->user; combo->user.str= sctx->user;
VOID(pthread_mutex_lock(&acl_cache->lock));
if (!find_acl_user(combo->host.str=(char*)sctx->host_or_ip, combo->user.str, if (!find_acl_user(combo->host.str=(char*)sctx->host_or_ip, combo->user.str,
FALSE) && FALSE) &&
!find_acl_user(combo->host.str=(char*)sctx->host, combo->user.str, !find_acl_user(combo->host.str=(char*)sctx->host, combo->user.str,
...@@ -5613,7 +5603,11 @@ bool sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name, ...@@ -5613,7 +5603,11 @@ bool sp_grant_privileges(THD *thd, const char *sp_db, const char *sp_name,
!find_acl_user(combo->host.str=(char*)sctx->ip, combo->user.str, !find_acl_user(combo->host.str=(char*)sctx->ip, combo->user.str,
FALSE) && FALSE) &&
!find_acl_user(combo->host.str=(char*)"%", combo->user.str, FALSE)) !find_acl_user(combo->host.str=(char*)"%", combo->user.str, FALSE))
{
VOID(pthread_mutex_unlock(&acl_cache->lock));
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
}
VOID(pthread_mutex_unlock(&acl_cache->lock));
bzero((char*)tables, sizeof(TABLE_LIST)); bzero((char*)tables, sizeof(TABLE_LIST));
user_list.empty(); user_list.empty();
...@@ -5731,6 +5725,8 @@ int fill_schema_user_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5731,6 +5725,8 @@ int fill_schema_user_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
char *curr_host= thd->security_ctx->priv_host_name(); char *curr_host= thd->security_ctx->priv_host_name();
DBUG_ENTER("fill_schema_user_privileges"); DBUG_ENTER("fill_schema_user_privileges");
pthread_mutex_lock(&acl_cache->lock);
for (counter=0 ; counter < acl_users.elements ; counter++) for (counter=0 ; counter < acl_users.elements ; counter++)
{ {
const char *user,*host, *is_grantable="YES"; const char *user,*host, *is_grantable="YES";
...@@ -5766,6 +5762,9 @@ int fill_schema_user_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5766,6 +5762,9 @@ int fill_schema_user_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
} }
} }
} }
pthread_mutex_unlock(&acl_cache->lock);
DBUG_RETURN(0); DBUG_RETURN(0);
#else #else
return(0); return(0);
...@@ -5785,6 +5784,8 @@ int fill_schema_schema_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5785,6 +5784,8 @@ int fill_schema_schema_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
char *curr_host= thd->security_ctx->priv_host_name(); char *curr_host= thd->security_ctx->priv_host_name();
DBUG_ENTER("fill_schema_schema_privileges"); DBUG_ENTER("fill_schema_schema_privileges");
pthread_mutex_lock(&acl_cache->lock);
for (counter=0 ; counter < acl_dbs.elements ; counter++) for (counter=0 ; counter < acl_dbs.elements ; counter++)
{ {
const char *user, *host, *is_grantable="YES"; const char *user, *host, *is_grantable="YES";
...@@ -5823,6 +5824,9 @@ int fill_schema_schema_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5823,6 +5824,9 @@ int fill_schema_schema_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
} }
} }
} }
pthread_mutex_unlock(&acl_cache->lock);
DBUG_RETURN(0); DBUG_RETURN(0);
#else #else
return (0); return (0);
...@@ -5840,6 +5844,8 @@ int fill_schema_table_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5840,6 +5844,8 @@ int fill_schema_table_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
char *curr_host= thd->security_ctx->priv_host_name(); char *curr_host= thd->security_ctx->priv_host_name();
DBUG_ENTER("fill_schema_table_privileges"); DBUG_ENTER("fill_schema_table_privileges");
rw_rdlock(&LOCK_grant);
for (index=0 ; index < column_priv_hash.records ; index++) for (index=0 ; index < column_priv_hash.records ; index++)
{ {
const char *user, *is_grantable= "YES"; const char *user, *is_grantable= "YES";
...@@ -5885,6 +5891,9 @@ int fill_schema_table_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5885,6 +5891,9 @@ int fill_schema_table_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
} }
} }
} }
rw_unlock(&LOCK_grant);
DBUG_RETURN(0); DBUG_RETURN(0);
#else #else
return (0); return (0);
...@@ -5902,6 +5911,8 @@ int fill_schema_column_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5902,6 +5911,8 @@ int fill_schema_column_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
char *curr_host= thd->security_ctx->priv_host_name(); char *curr_host= thd->security_ctx->priv_host_name();
DBUG_ENTER("fill_schema_table_privileges"); DBUG_ENTER("fill_schema_table_privileges");
rw_rdlock(&LOCK_grant);
for (index=0 ; index < column_priv_hash.records ; index++) for (index=0 ; index < column_priv_hash.records ; index++)
{ {
const char *user, *is_grantable= "YES"; const char *user, *is_grantable= "YES";
...@@ -5953,6 +5964,9 @@ int fill_schema_column_privileges(THD *thd, TABLE_LIST *tables, COND *cond) ...@@ -5953,6 +5964,9 @@ int fill_schema_column_privileges(THD *thd, TABLE_LIST *tables, COND *cond)
} }
} }
} }
rw_unlock(&LOCK_grant);
DBUG_RETURN(0); DBUG_RETURN(0);
#else #else
return (0); return (0);
......
sql/sql_acl.h
View file @ 3d45a067
...@@ -196,7 +196,6 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list, ...@@ -196,7 +196,6 @@ bool mysql_table_grant(THD *thd, TABLE_LIST *table, List <LEX_USER> &user_list,
bool mysql_routine_grant(THD *thd, TABLE_LIST *table, bool is_proc, bool mysql_routine_grant(THD *thd, TABLE_LIST *table, bool is_proc,
List <LEX_USER> &user_list, ulong rights, List <LEX_USER> &user_list, ulong rights,
bool revoke, bool no_error); bool revoke, bool no_error);
ACL_USER *check_acl_user(LEX_USER *user_name, uint *acl_acl_userdx);
my_bool grant_init(); my_bool grant_init();
void grant_free(void); void grant_free(void);
my_bool grant_reload(THD *thd); my_bool grant_reload(THD *thd);
......
sql/sql_parse.cc
View file @ 3d45a067
...@@ -3835,7 +3835,6 @@ mysql_execute_command(THD *thd) ...@@ -3835,7 +3835,6 @@ mysql_execute_command(THD *thd)
if (thd->security_ctx->user) // If not replication if (thd->security_ctx->user) // If not replication
{ {
LEX_USER *user; LEX_USER *user;
uint counter;
List_iterator <LEX_USER> user_list(lex->users_list); List_iterator <LEX_USER> user_list(lex->users_list);
while ((user= user_list++)) while ((user= user_list++))
...@@ -3853,7 +3852,8 @@ mysql_execute_command(THD *thd) ...@@ -3853,7 +3852,8 @@ mysql_execute_command(THD *thd)
user->host.str, thd->security_ctx->host_or_ip)) user->host.str, thd->security_ctx->host_or_ip))
{ {
// TODO: use check_change_password() // TODO: use check_change_password()
if (check_acl_user(user, &counter) && user->password.str && if (is_acl_user(user->host.str, user->user.str) &&
user->password.str &&
check_access(thd, UPDATE_ACL,"mysql",0,1,1,0)) check_access(thd, UPDATE_ACL,"mysql",0,1,1,0))
{ {
my_message(ER_PASSWORD_NOT_ALLOWED, my_message(ER_PASSWORD_NOT_ALLOWED,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7