diff options
| -rw-r--r-- | mysql-test/suite/roles/rebuild_role_grants.result | 266 | ||||
| -rw-r--r-- | mysql-test/suite/roles/rebuild_role_grants.test | 31 | ||||
| -rw-r--r-- | sql/sql_acl.cc | 371 |
3 files changed, 479 insertions, 189 deletions
diff --git a/mysql-test/suite/roles/rebuild_role_grants.result b/mysql-test/suite/roles/rebuild_role_grants.result index 72eabe38b93..101efd47569 100644 --- a/mysql-test/suite/roles/rebuild_role_grants.result +++ b/mysql-test/suite/roles/rebuild_role_grants.result @@ -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; diff --git a/mysql-test/suite/roles/rebuild_role_grants.test b/mysql-test/suite/roles/rebuild_role_grants.test index 84dbdf78fb8..7007df0ecdd 100644 --- a/mysql-test/suite/roles/rebuild_role_grants.test +++ b/mysql-test/suite/roles/rebuild_role_grants.test @@ -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; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 89fecc92e9b..f62dd5471eb 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -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 |