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

[John Johansen]

On 05/22/2013 07:40 PM, Seth Arnold wrote:
> 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.
> 
I actually have a kernel notes doc that covers this and a lot more
(AppArmor Develper 1 - Kernel Notes.odt in bzr), however it hasn't been
updated for the last couple months so it isn't up to date on all the changes

>> @@ -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...
> 
hehe, yeah the label is the refcount object

>> @@ -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?
> 
uh no they won't be, not that that makes difference here

however atm this is just place holder code for more changes to come. At
this point there is only one profile within the label set so first and last
are very much the same.


>> @@ -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. :)
> 
yes there are more patches on top of this and well every thing else, you
haven't even hit the first of the label_for_each patches