Follow up to MDEV-12366: FLUSH privileges can break hierarchy of roles

A suggestion to make role propagation simpler from serg@mariadb.org.

Instead of gathering the leaf roles in an array, which for very wide
graphs could potentially mean a big part of the whole roles schema, keep
the previous logic. When finally merging a role, set its counter
to something positive.

This will effectively mean that a role has been merged, thus a random pass
through roles hash that touches a previously merged role won't cause the problem
described in MDEV-12366 any more, as propagate_role_grants_action will stop
attempting to merge from that role.

1 files changed, 11 insertions, 15 deletions

diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 27292105dde..0e4a0cef591 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -5366,6 +5366,8 @@ static int merge_role_privileges(ACL_ROLE *role __attribute__((unused)),
   if (--grantee->counter)
     return 1; // don't recurse into grantee just yet
 
+  grantee->counter= 1; // Mark the grantee as merged.
+
   /* if we'll do db/table/routine privileges, create a hash of role names */
   role_hash_t role_hash(role_key);
   if (data->what != PRIVS_TO_MERGE::GLOBAL)
@@ -6555,14 +6557,16 @@ end_index_init:
   DBUG_RETURN(return_val);
 }
 
-static my_bool collect_leaf_roles(void *role_ptr,
-                                  void *roles_array)
+static my_bool propagate_role_grants_action(void *role_ptr,
+                                            void *ptr __attribute__((unused)))
 {
   ACL_ROLE *role= static_cast<ACL_ROLE *>(role_ptr);
-  Dynamic_array<ACL_ROLE *> *array=
-    static_cast<Dynamic_array<ACL_ROLE *> *>(roles_array);
-  if (!role->counter)
-    array->push(role);
+  if (role->counter)
+    return 0;
+
+  mysql_mutex_assert_owner(&acl_cache->lock);
+  PRIVS_TO_MERGE data= { PRIVS_TO_MERGE::ALL, 0, 0 };
+  traverse_role_graph_up(role, &data, NULL, merge_role_privileges);
   return 0;
 }
 
@@ -6645,15 +6649,7 @@ my_bool grant_reload(THD *thd)
   }
 
   mysql_mutex_lock(&acl_cache->lock);
-  Dynamic_array<ACL_ROLE *> leaf_roles;
-  my_hash_iterate(&acl_roles, collect_leaf_roles, &leaf_roles);
-  PRIVS_TO_MERGE data= { PRIVS_TO_MERGE::ALL, 0, 0 };
-  for (size_t i= 0; i < leaf_roles.elements(); i++)
-  {
-    traverse_role_graph_up(leaf_roles.at(i), &data, NULL,
-                           merge_role_privileges);
-  }
-
+  my_hash_iterate(&acl_roles, propagate_role_grants_action, NULL);
   mysql_mutex_unlock(&acl_cache->lock);
 
   mysql_rwlock_unlock(&LOCK_grant);