[apparmor] [PATCH 23/36] apparmor: introduce using labels from contexts

[Seth Arnold]

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.johansen at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130522/ea355229/attachment.pgp>