On Wed, May 01, 2013 at 02:31:08PM -0700, John Johansen wrote: > Baby step to using labels instead of profiles. Switch from using profile > refs to label refs. Note this step does not make any functional changes > > Signed-off-by: John Johansen <john.johan...@canonical.com>
A few small comments inline..
>
> /**
> - * aa_get_task_profile - Get another task's profile
> + * aa_get_task_label - Get another task's label
> * @task: task to query (NOT NULL)
> *
> - * Returns: counted reference to @task's profile
> + * Returns: counted reference to @task's label
> */
> -struct aa_profile *aa_get_task_profile(struct task_struct *task)
> +struct aa_label *aa_get_task_label(struct task_struct *task)
> {
> - struct aa_profile *p;
> + struct aa_label *p;
>
> rcu_read_lock();
> - p = aa_get_profile(__aa_task_profile(task));
> + p = aa_get_label(__aa_task_label(task));
> rcu_read_unlock();
>
> return p;
> }
I'd find it useful to have a new guide to our refcounting and what
changes as part of this transition. This one change seems to indicate
that labels are the new ref-counted things, but that doesn't feel
sufficient.
> @@ -381,11 +383,11 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
> /* Test for onexec first as onexec directives override other
> * x transitions.
> */
> - if (unconfined(profile)) {
> + if (profile_unconfined(profile)) {
> /* unconfined task */
> if (cxt->onexec)
> /* change_profile on exec already been granted */
> - new_profile = aa_get_profile(cxt->onexec);
> + new_profile = labels_profile(aa_get_label(cxt->onexec));
> else
> new_profile = find_attach(ns, &ns->base.profiles, name);
> if (!new_profile)
This section is why I wanted an updated refcount guide...
> @@ -411,13 +413,13 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
> * exec\0change_profile
> */
> state = aa_dfa_null_transition(profile->file.dfa, state);
> - cp = change_profile_perms(profile, cxt->onexec->ns,
> - cxt->onexec->base.name,
> + cp = change_profile_perms(profile,
> labels_profile(cxt->onexec)->ns,
> +
> labels_profile(cxt->onexec)->base.name,
> AA_MAY_ONEXEC, state);
>
You could use labels_ns() here instead of the labels_profile()->ns. It'd
get the ns from the last profile rather than the first profile, but the
intention is for them all to be identical, right?
> @@ -103,27 +103,31 @@ int aa_task_setrlimit(struct aa_profile *profile,
> struct task_struct *task,
> * that the task is setting the resource of a task confined with
> * the same profile.
> */
> - if (profile != task_profile ||
> - (profile->rlimits.mask & (1 << resource) &&
> - new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max))
> + if (label != task_label ||
> + (labels_profile(label)->rlimits.mask & (1 << resource) &&
> + new_rlim->rlim_max >
> labels_profile(label)->rlimits.limits[resource].rlim_max))
> error = -EACCES;
Oh. Hrm. I have a feeling this code will need to be revisted before this
transition is over.. probably not in this patch, but hopefully soon. :)
Thanks
signature.asc
Description: Digital signature
-- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
