Skip to content

Commit

Permalink
Follow up to MDEV-12366: FLUSH privileges can break hierarchy of roles
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cvicentiu committed Dec 21, 2017
1 parent 0202e47 commit 24efee9
Showing 1 changed file with 11 additions and 15 deletions.
26 changes: 11 additions & 15 deletions sql/sql_acl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -6555,14 +6557,16 @@ static my_bool grant_load(THD *thd, TABLE_LIST *tables)
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;
}

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 24efee9

Please sign in to comment.