On Fri, Feb 08, 2013 at 01:00:55PM -0800, John Johansen wrote:
> +/**
> + * aa_get_profile_rcu - increment a refcount profile that can be replaced
> + * @p: pointer to profile that can be replaced (NOT NULL)
> + *
> + * Returns: pointer to a refcounted profile.
> + * else NULL if no profile
> + */
> +static inline struct aa_profile *aa_get_profile_rcu(struct aa_profile **p)
> +{
> + struct aa_profile *c;
> +
> + rcu_read_lock();
> + do {
> + c = rcu_dereference(*p);
> + } while (c && !kref_get_not0(&c->base.count));
> + rcu_read_unlock();
> +
> + return c;
> +}
The only use of this function is to get profile->parent in change_hat.
Does profile->parent need this extra protection in the other places it
is used? Or is it sufficient to just deref parent without getting its
reference count in the other cases? (filesystem, freeing, new null
profile).
> @@ -447,7 +433,7 @@ out:
> static void __list_add_profile(struct list_head *list,
> struct aa_profile *profile)
> {
> - list_add(&profile->base.list, list);
> + list_add_rcu(&profile->base.list, list);
> /* get list reference */
> aa_get_profile(profile);
> }
Two of the three callers of this function have an aa_get_profile(profile);
equivalent line very close by. Are the callers mistaken? Is this routine
mistaken? Or are they weird because the _third_ case, replacement, is
very weird? :)
More later; this one's big. :)
Thanks John
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
