Skip to content

M
MariaDB
Commit 9e6c3838 authored by Vicențiu Ciorbaru's avatar Vicențiu Ciorbaru Committed by Vicențiu-Marian Ciorbaru
Browse files
Options

MDEV-17964: Assertion `status == 0' failed in add_role_user_mapping_action 

This happens upon CREATE USER and DROP ROLE.

The underlying problem is that our HASH implementation shuffles elements
around when performing an update or delete. This means that when doing a
scan through the HASH table by index, in search of elements to delete or
update one must restart the scan to make sure nothing is missed if at least
one delete / update happened.

More specifically, what happened in this case:
The hash has 131 element, DROP ROLE removes the element
[119]. Its [119]->next was element [129], so [129] is moved to [119].
Now we need to compact the hash, removing the last element [130]. It
gets one bit off its hash value and becomes element [2]. The existing
element [2] is moved to [129], and old [130] is moved to [2].

We cannot simply move [130] to [129] and make [2]->next=130, it won't
work if [2] is itself in the collision list and doesn't belong in [2].

The handle_grant_struct code assumed that it is safe to continue by only
reexamining the currently modified / deleted element index, but that is
not true.

Missing to delete an element in the hash triggered the assertion in
the test case. DROP ROLE would not clear all necessary role->role or
role->user mappings.

To fix the problem we ensure that the scan is restarted, only if an
element was deleted / updated, similar to how bubble-sort keeps sorting
until it finds no more elements to swap.
parent a2a42f4e
Hide whitespace changes
Inline Side-by-side
Showing with 479 additions and 189 deletions
mysql-test/suite/roles/rebuild_role_grants.result
View file @ 9e6c3838
......@@ -65,3 +65,269 @@ drop role look, isp, xxx, ppp;
connection default;
disconnect con1;
drop user nnnn@'%';
CREATE USER u@localhost;
CREATE ROLE r1;
CREATE ROLE r2;
CREATE ROLE r3;
CREATE ROLE r4;
CREATE ROLE r5;
CREATE ROLE r6;
CREATE ROLE r7;
CREATE ROLE r8;
CREATE ROLE r9;
CREATE ROLE r10;
CREATE ROLE r11;
CREATE ROLE r12;
CREATE ROLE r13;
CREATE ROLE r14;
CREATE ROLE r15;
CREATE ROLE r16;
CREATE ROLE r17;
CREATE ROLE r18;
CREATE ROLE r19;
CREATE ROLE r20;
CREATE ROLE r21;
CREATE ROLE r22;
CREATE ROLE r23;
CREATE ROLE r24;
CREATE ROLE r25;
CREATE ROLE r26;
CREATE ROLE r27;
CREATE ROLE r28;
CREATE ROLE r29;
CREATE ROLE r30;
CREATE ROLE r31;
CREATE ROLE r32;
CREATE ROLE r33;
CREATE ROLE r34;
CREATE ROLE r35;
CREATE ROLE r36;
CREATE ROLE r37;
CREATE ROLE r38;
CREATE ROLE r39;
CREATE ROLE r40;
CREATE ROLE r41;
CREATE ROLE r42;
CREATE ROLE r43;
CREATE ROLE r44;
CREATE ROLE r45;
CREATE ROLE r46;
CREATE ROLE r47;
CREATE ROLE r48;
CREATE ROLE r49;
CREATE ROLE r50;
CREATE ROLE r51;
CREATE ROLE r52;
CREATE ROLE r53;
CREATE ROLE r54;
CREATE ROLE r55;
CREATE ROLE r56;
CREATE ROLE r57;
CREATE ROLE r58;
CREATE ROLE r59;
CREATE ROLE r60;
CREATE ROLE r61;
CREATE ROLE r62;
CREATE ROLE r63;
CREATE ROLE r64;
CREATE ROLE r65;
CREATE ROLE r66;
CREATE ROLE r67;
CREATE ROLE r68;
CREATE ROLE r69;
CREATE ROLE r70;
CREATE ROLE r71;
CREATE ROLE r72;
CREATE ROLE r73;
CREATE ROLE r74;
CREATE ROLE r75;
CREATE ROLE r76;
CREATE ROLE r77;
CREATE ROLE r78;
CREATE ROLE r79;
CREATE ROLE r80;
CREATE ROLE r81;
CREATE ROLE r82;
CREATE ROLE r83;
CREATE ROLE r84;
CREATE ROLE r85;
CREATE ROLE r86;
CREATE ROLE r87;
CREATE ROLE r88;
CREATE ROLE r89;
CREATE ROLE r90;
CREATE ROLE r91;
CREATE ROLE r92;
CREATE ROLE r93;
CREATE ROLE r94;
CREATE ROLE r95;
CREATE ROLE r96;
CREATE ROLE r97;
CREATE ROLE r98;
CREATE ROLE r99;
CREATE ROLE r100;
CREATE ROLE r101;
CREATE ROLE r102;
CREATE ROLE r103;
CREATE ROLE r104;
CREATE ROLE r105;
CREATE ROLE r106;
CREATE ROLE r107;
CREATE ROLE r108;
CREATE ROLE r109;
CREATE ROLE r110;
CREATE ROLE r111;
CREATE ROLE r112;
CREATE ROLE r113;
CREATE ROLE r114;
CREATE ROLE r115;
CREATE ROLE r116;
CREATE ROLE r117;
CREATE ROLE r118;
CREATE ROLE r119;
CREATE ROLE r120;
CREATE ROLE r121;
CREATE ROLE r122;
CREATE ROLE r123;
CREATE ROLE r124;
CREATE ROLE r125;
CREATE ROLE r126;
CREATE ROLE r127;
CREATE ROLE r128;
CREATE ROLE n;
CREATE ROLE d WITH ADMIN n;
CREATE ROLE '%' WITH ADMIN u@localhost;
DROP ROLE n;
CREATE USER 't';
DROP ROLE r1;
DROP ROLE r2;
DROP ROLE r3;
DROP ROLE r4;
DROP ROLE r5;
DROP ROLE r6;
DROP ROLE r7;
DROP ROLE r8;
DROP ROLE r9;
DROP ROLE r10;
DROP ROLE r11;
DROP ROLE r12;
DROP ROLE r13;
DROP ROLE r14;
DROP ROLE r15;
DROP ROLE r16;
DROP ROLE r17;
DROP ROLE r18;
DROP ROLE r19;
DROP ROLE r20;
DROP ROLE r21;
DROP ROLE r22;
DROP ROLE r23;
DROP ROLE r24;
DROP ROLE r25;
DROP ROLE r26;
DROP ROLE r27;
DROP ROLE r28;
DROP ROLE r29;
DROP ROLE r30;
DROP ROLE r31;
DROP ROLE r32;
DROP ROLE r33;
DROP ROLE r34;
DROP ROLE r35;
DROP ROLE r36;
DROP ROLE r37;
DROP ROLE r38;
DROP ROLE r39;
DROP ROLE r40;
DROP ROLE r41;
DROP ROLE r42;
DROP ROLE r43;
DROP ROLE r44;
DROP ROLE r45;
DROP ROLE r46;
DROP ROLE r47;
DROP ROLE r48;
DROP ROLE r49;
DROP ROLE r50;
DROP ROLE r51;
DROP ROLE r52;
DROP ROLE r53;
DROP ROLE r54;
DROP ROLE r55;
DROP ROLE r56;
DROP ROLE r57;
DROP ROLE r58;
DROP ROLE r59;
DROP ROLE r60;
DROP ROLE r61;
DROP ROLE r62;
DROP ROLE r63;
DROP ROLE r64;
DROP ROLE r65;
DROP ROLE r66;
DROP ROLE r67;
DROP ROLE r68;
DROP ROLE r69;
DROP ROLE r70;
DROP ROLE r71;
DROP ROLE r72;
DROP ROLE r73;
DROP ROLE r74;
DROP ROLE r75;
DROP ROLE r76;
DROP ROLE r77;
DROP ROLE r78;
DROP ROLE r79;
DROP ROLE r80;
DROP ROLE r81;
DROP ROLE r82;
DROP ROLE r83;
DROP ROLE r84;
DROP ROLE r85;
DROP ROLE r86;
DROP ROLE r87;
DROP ROLE r88;
DROP ROLE r89;
DROP ROLE r90;
DROP ROLE r91;
DROP ROLE r92;
DROP ROLE r93;
DROP ROLE r94;
DROP ROLE r95;
DROP ROLE r96;
DROP ROLE r97;
DROP ROLE r98;
DROP ROLE r99;
DROP ROLE r100;
DROP ROLE r101;
DROP ROLE r102;
DROP ROLE r103;
DROP ROLE r104;
DROP ROLE r105;
DROP ROLE r106;
DROP ROLE r107;
DROP ROLE r108;
DROP ROLE r109;
DROP ROLE r110;
DROP ROLE r111;
DROP ROLE r112;
DROP ROLE r113;
DROP ROLE r114;
DROP ROLE r115;
DROP ROLE r116;
DROP ROLE r117;
DROP ROLE r118;
DROP ROLE r119;
DROP ROLE r120;
DROP ROLE r121;
DROP ROLE r122;
DROP ROLE r123;
DROP ROLE r124;
DROP ROLE r125;
DROP ROLE r126;
DROP ROLE r127;
DROP ROLE r128;
DROP ROLE d;
DROP ROLE '%';
DROP USER 't';
DROP USER u@localhost;
mysql-test/suite/roles/rebuild_role_grants.test
View file @ 9e6c3838
......@@ -67,3 +67,34 @@ drop role look, isp, xxx, ppp;
connection default;
disconnect con1;
drop user nnnn@'%';
#
# MDEV-17964 Assertion `status == 0' failed in add_role_user_mapping_action
# upon CREATE USER and DROP ROLE
#
CREATE USER u@localhost;
--let $n= 1
while ($n < 129)
{
eval CREATE ROLE r$n;
inc $n;
}
CREATE ROLE n;
CREATE ROLE d WITH ADMIN n;
CREATE ROLE '%' WITH ADMIN u@localhost;
DROP ROLE n;
CREATE USER 't';
--let $n= 1
while ($n < 129)
{
eval DROP ROLE r$n;
inc $n;
}
DROP ROLE d;
DROP ROLE '%';
DROP USER 't';
DROP USER u@localhost;
sql/sql_acl.cc
View file @ 9e6c3838
......@@ -9645,8 +9645,8 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
LEX_USER *user_from, LEX_USER *user_to)
{
int result= 0;
int idx;
int elements;
bool restart;
const char *UNINIT_VAR(user);
const char *UNINIT_VAR(host);
ACL_USER *acl_user= NULL;
......@@ -9747,82 +9747,98 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
DBUG_RETURN(-1);
}
#ifdef EXTRA_DEBUG
DBUG_PRINT("loop",("scan struct: %u search user: '%s' host: '%s'",
struct_no, user_from->user.str, user_from->host.str));
#endif
/* Loop over all elements *backwards* (see the comment below). */
for (idx= elements - 1; idx >= 0; idx--)
{
/*
Get a pointer to the element.
*/
switch (struct_no) {
case USER_ACL:
acl_user= dynamic_element(&acl_users, idx, ACL_USER*);
user= acl_user->user.str;
host= acl_user->host.hostname;
break;
/* Loop over elements backwards as it may reduce the number of mem-moves
for dynamic arrays.
case DB_ACL:
acl_db= &acl_dbs.at(idx);
user= acl_db->user;
host= acl_db->host.hostname;
We restart the loop, if we deleted or updated anything in a hash table
because calling my_hash_delete or my_hash_update shuffles elements indices
and we can miss some if we do only one scan.
*/
do {
restart= false;
for (int idx= elements - 1; idx >= 0; idx--)
{
/*
Get a pointer to the element.
*/
switch (struct_no) {
case USER_ACL:
acl_user= dynamic_element(&acl_users, idx, ACL_USER*);
user= acl_user->user.str;
host= acl_user->host.hostname;
break;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
grant_name= (GRANT_NAME*) my_hash_element(grant_name_hash, idx);
user= grant_name->user;
host= grant_name->host.hostname;
break;
case DB_ACL:
acl_db= &acl_dbs.at(idx);
user= acl_db->user;
host= acl_db->host.hostname;
break;
case PROXY_USERS_ACL:
acl_proxy_user= dynamic_element(&acl_proxy_users, idx, ACL_PROXY_USER*);
user= acl_proxy_user->get_user();
host= acl_proxy_user->get_host();
break;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
grant_name= (GRANT_NAME*) my_hash_element(grant_name_hash, idx);
user= grant_name->user;
host= grant_name->host.hostname;
break;
case ROLES_MAPPINGS_HASH:
role_grant_pair= (ROLE_GRANT_PAIR *) my_hash_element(roles_mappings_hash, idx);
user= role_grant_pair->u_uname;
host= role_grant_pair->u_hname;
break;
case PROXY_USERS_ACL:
acl_proxy_user= dynamic_element(&acl_proxy_users, idx, ACL_PROXY_USER*);
user= acl_proxy_user->get_user();
host= acl_proxy_user->get_host();
break;
default:
DBUG_ASSERT(0);
}
if (! user)
user= "";
if (! host)
host= "";
case ROLES_MAPPINGS_HASH:
role_grant_pair= (ROLE_GRANT_PAIR *) my_hash_element(roles_mappings_hash, idx);
user= role_grant_pair->u_uname;
host= role_grant_pair->u_hname;
break;
default:
DBUG_ASSERT(0);
}
if (! user)
user= "";
if (! host)
host= "";
#ifdef EXTRA_DEBUG
DBUG_PRINT("loop",("scan struct: %u index: %u user: '%s' host: '%s'",
struct_no, idx, user, host));
DBUG_PRINT("loop",("scan struct: %u index: %u user: '%s' host: '%s'",
struct_no, idx, user, host));
#endif
if (struct_no == ROLES_MAPPINGS_HASH)
{
const char* role= role_grant_pair->r_uname? role_grant_pair->r_uname: "";
if (user_from->is_role())
if (struct_no == ROLES_MAPPINGS_HASH)
{
/* When searching for roles within the ROLES_MAPPINGS_HASH, we have
to check both the user field as well as the role field for a match.
const char* role= role_grant_pair->r_uname? role_grant_pair->r_uname: "";
if (user_from->is_role())
{
/* When searching for roles within the ROLES_MAPPINGS_HASH, we have
to check both the user field as well as the role field for a match.
It is possible to have a role granted to a role. If we are going
to modify the mapping entry, it needs to be done on either on the
"user" end (here represented by a role) or the "role" end. At least
one part must match.
It is possible to have a role granted to a role. If we are going
to modify the mapping entry, it needs to be done on either on the
"user" end (here represented by a role) or the "role" end. At least
one part must match.
If the "user" end has a not-empty host string, it can never match
as we are searching for a role here. A role always has an empty host
string.
*/
if ((*host || strcmp(user_from->user.str, user)) &&
strcmp(user_from->user.str, role))
continue;
If the "user" end has a not-empty host string, it can never match
as we are searching for a role here. A role always has an empty host
string.
*/
if ((*host || strcmp(user_from->user.str, user)) &&
strcmp(user_from->user.str, role))
continue;
}
else
{
if (strcmp(user_from->user.str, user) ||
my_strcasecmp(system_charset_info, user_from->host.str, host))
continue;
}
}
else
{
......@@ -9830,154 +9846,131 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
my_strcasecmp(system_charset_info, user_from->host.str, host))
continue;
}
}
else
{
if (strcmp(user_from->user.str, user) ||
my_strcasecmp(system_charset_info, user_from->host.str, host))
continue;
}
result= 1; /* At least one element found. */
if ( drop )
{
elements--;
switch ( struct_no ) {
case USER_ACL:
free_acl_user(dynamic_element(&acl_users, idx, ACL_USER*));
delete_dynamic_element(&acl_users, idx);
break;
result= 1; /* At least one element found. */
if ( drop )
{
elements--;
switch ( struct_no ) {
case USER_ACL:
free_acl_user(dynamic_element(&acl_users, idx, ACL_USER*));
delete_dynamic_element(&acl_users, idx);
break;
case DB_ACL:
acl_dbs.del(idx);
break;
case DB_ACL:
acl_dbs.del(idx);
break;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
my_hash_delete(grant_name_hash, (uchar*) grant_name);
/*
In our HASH implementation on deletion one elements
is moved into a place where a deleted element was,
and the last element is moved into the empty space.
Thus we need to re-examine the current element, but
we don't have to restart the search from the beginning.
*/
if (idx != elements)
idx++;
break;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
my_hash_delete(grant_name_hash, (uchar*) grant_name);
restart= true;
break;
case PROXY_USERS_ACL:
delete_dynamic_element(&acl_proxy_users, idx);
break;
case PROXY_USERS_ACL:
delete_dynamic_element(&acl_proxy_users, idx);
break;
case ROLES_MAPPINGS_HASH:
my_hash_delete(roles_mappings_hash, (uchar*) role_grant_pair);
if (idx != elements)
idx++;
break;
case ROLES_MAPPINGS_HASH:
my_hash_delete(roles_mappings_hash, (uchar*) role_grant_pair);
restart= true;
break;
default:
DBUG_ASSERT(0);
break;
default:
DBUG_ASSERT(0);
break;
}
}
}
else if ( user_to )
{
switch ( struct_no ) {
case USER_ACL:
acl_user->user.str= strdup_root(&acl_memroot, user_to->user.str);
acl_user->user.length= user_to->user.length;
update_hostname(&acl_user->host, strdup_root(&acl_memroot, user_to->host.str));
acl_user->hostname_length= strlen(acl_user->host.hostname);
break;
else if ( user_to )
{
switch ( struct_no ) {
case USER_ACL:
acl_user->user.str= strdup_root(&acl_memroot, user_to->user.str);
acl_user->user.length= user_to->user.length;
update_hostname(&acl_user->host, strdup_root(&acl_memroot, user_to->host.str));
acl_user->hostname_length= strlen(acl_user->host.hostname);
break;
case DB_ACL:
acl_db->user= strdup_root(&acl_memroot, user_to->user.str);
update_hostname(&acl_db->host, strdup_root(&acl_memroot, user_to->host.str));
break;
case DB_ACL:
acl_db->user= strdup_root(&acl_memroot, user_to->user.str);
update_hostname(&acl_db->host, strdup_root(&acl_memroot, user_to->host.str));
break;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
{
/*
Save old hash key and its length to be able to properly update
element position in hash.
*/
char *old_key= grant_name->hash_key;
size_t old_key_length= grant_name->key_length;
case COLUMN_PRIVILEGES_HASH:
case PROC_PRIVILEGES_HASH:
case FUNC_PRIVILEGES_HASH:
{
/*
Save old hash key and its length to be able to properly update
element position in hash.
*/
char *old_key= grant_name->hash_key;
size_t old_key_length= grant_name->key_length;
/*
Update the grant structure with the new user name and host name.
*/
grant_name->set_user_details(user_to->host.str, grant_name->db,
user_to->user.str, grant_name->tname,
TRUE);
/*
Since username is part of the hash key, when the user name
is renamed, the hash key is changed. Update the hash to
ensure that the position matches the new hash key value
*/
my_hash_update(grant_name_hash, (uchar*) grant_name, (uchar*) old_key,
old_key_length);
restart= true;
break;
}
/*
Update the grant structure with the new user name and host name.
*/
grant_name->set_user_details(user_to->host.str, grant_name->db,
user_to->user.str, grant_name->tname,
TRUE);
/*
Since username is part of the hash key, when the user name
is renamed, the hash key is changed. Update the hash to
ensure that the position matches the new hash key value
*/
my_hash_update(grant_name_hash, (uchar*) grant_name, (uchar*) old_key,
old_key_length);
/*
hash_update() operation could have moved element from the tail or
the head of the hash to the current position. But it can never
move an element from the head to the tail or from the tail to the
head over the current element.
So we need to examine the current element once again, but
we don't need to restart the search from the beginning.
*/
idx++;
case PROXY_USERS_ACL:
acl_proxy_user->set_user (&acl_memroot, user_to->user.str);
acl_proxy_user->set_host (&acl_memroot, user_to->host.str);
break;
}
case PROXY_USERS_ACL:
acl_proxy_user->set_user (&acl_memroot, user_to->user.str);
acl_proxy_user->set_host (&acl_memroot, user_to->host.str);
break;
case ROLES_MAPPINGS_HASH:
{
/*
Save old hash key and its length to be able to properly update
element position in hash.
*/
char *old_key= role_grant_pair->hashkey.str;
size_t old_key_length= role_grant_pair->hashkey.length;
bool oom;
if (user_to->is_role())
oom= role_grant_pair->init(&acl_memroot, role_grant_pair->u_uname,
role_grant_pair->u_hname,
user_to->user.str, false);
else
oom= role_grant_pair->init(&acl_memroot, user_to->user.str,
user_to->host.str,
role_grant_pair->r_uname, false);
if (oom)
DBUG_RETURN(-1);
my_hash_update(roles_mappings_hash, (uchar*) role_grant_pair,
(uchar*) old_key, old_key_length);
restart= true;
break;
}
case ROLES_MAPPINGS_HASH:
{
/*
Save old hash key and its length to be able to properly update
element position in hash.
*/
char *old_key= role_grant_pair->hashkey.str;
size_t old_key_length= role_grant_pair->hashkey.length;
bool oom;
if (user_to->is_role())
oom= role_grant_pair->init(&acl_memroot, role_grant_pair->u_uname,
role_grant_pair->u_hname,
user_to->user.str, false);
else
oom= role_grant_pair->init(&acl_memroot, user_to->user.str,
user_to->host.str,
role_grant_pair->r_uname, false);
if (oom)
DBUG_RETURN(-1);
my_hash_update(roles_mappings_hash, (uchar*) role_grant_pair,
(uchar*) old_key, old_key_length);
idx++; // see the comment above
default:
DBUG_ASSERT(0);
break;
}
default:
DBUG_ASSERT(0);
}
else
{
/* If search is requested, we do not need to search further. */
break;
}
}
else
{
/* If search is requested, we do not need to search further. */
break;
}
}
} while (restart);
#ifdef EXTRA_DEBUG
DBUG_PRINT("loop",("scan struct: %u result %d", struct_no, result));
#endif
......
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