Re: [PATCH 2/4] AppArmor: Fix security_task_setrlimit logic for 2.6.36changes

[Jiri Slaby]

On 08/28/2010 10:35 PM, John Johansen wrote:
> On 08/28/2010 11:15 AM, Jiri Slaby wrote:
>> On 08/28/2010 07:10 PM, John Johansen wrote:
>>> 2.6.36 introduced the abilitiy to specify the task that is having its
>>> rlimits set.  Update mediation to ensure that confined tasks can only
>>> set their own group_leader as expected by current policy.
>>>
>>> Add TODO note about extending policy to support setting other tasks
>>> rlimits.
>> ...
>>> --- a/security/apparmor/resource.c
>>> +++ b/security/apparmor/resource.c
>> ...
>>> @@ -79,18 +80,21 @@ int aa_map_resource(int resource)
>>>   *
>>>   * Returns: 0 or error code if setting resource failed
>>>   */
>>> -int aa_task_setrlimit(struct aa_profile *profile, unsigned int resource,
>>> -		      struct rlimit *new_rlim)
>>> +int aa_task_setrlimit(struct aa_profile *profile, struct task_struct *task,
>>> +		      unsigned int resource, struct rlimit *new_rlim)
>>>  {
>>>  	int error = 0;
>>>  
>>> -	if (profile->rlimits.mask & (1 << resource) &&
>>> -	    new_rlim->rlim_max > profile->rlimits.limits[resource].rlim_max)
>>> -
>>> -		error = audit_resource(profile, resource, new_rlim->rlim_max,
>>> -			-EACCES);
>>> +	/* TODO: extend resource control to handle non group leader tasks.
>>> +	 * AppArmor rules currently have the implicit assumption that
>>> +	 * the task having its resource set is the group leader.
>>
>> Why would you want to do that? Limits are per process, so the 'task'
>> parameter is guaranteed to be the leader.
>>
> That used to be the case,

It is still the case. The limits (the same as signals or accounting) are
per-process, they are not per-thread. If you look into do_prlimit() how
security_task_setrlimit() is called, you'll see.

> commit c022a0acad534fd5f5d5f17280f6d4d135e74e81 add the prlimit64 syscall
> which
> 
>     It also adds a possibility of changing limits of other processes. We
>     check the user's permissions to do that and if it succeeds, the new
>     limits are propagated online.
...
> so it seems we need to extend the apparmor rules to be able to deal with
> this, but ensuring that the current assumption is enforced is enough
> for now.

Yeah, I remember, the other Jiri inside wrote that. You are guaranteed
to get the group leader right now. And if it ever changes, which is
unlikely, all users would have to be checked and fixed anyway.

regards,
-- 
js
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/