Re: RT scheduling and a way to make a process hang, unkillable

[Peter Zijlstra]

On Mon, 2009-02-16 at 16:06 +0530, Dhaval Giani wrote:

> sched: Don't allow setuid to succeed if the user does not have rt bandwidth
> 
> Corey Hickey reported that on using setuid to change the uid of a
> rt process, the process would be unkillable and not be running.
> This is because there was no rt runtime for that user group. Add
> in a check to see if a user can attach an rt task to its task group.
> 
> Disclaimer: Not sure about the return values, and if setuid allows
> return values other than EPERM and EAGAIN.
> 
> Not-Yet-Signed-off-by: Dhaval Giani <dhaval@xxxxxxxxxxxxxxxxxx>
> 
> Index: linux-2.6/include/linux/sched.h
> ===================================================================
> --- linux-2.6.orig/include/linux/sched.h
> +++ linux-2.6/include/linux/sched.h
> @@ -2320,9 +2320,12 @@ extern long sched_group_rt_runtime(struc
>  extern int sched_group_set_rt_period(struct task_group *tg,
>  				      long rt_period_us);
>  extern long sched_group_rt_period(struct task_group *tg);
> +int sched_rt_can_attach(struct task_group *tg, struct task_struct *tsk);
>  #endif
>  #endif
>  
> +int rt_task_can_switch_user(uid_t uid, struct task_struct *tsk);
> +
>  #ifdef CONFIG_TASK_XACCT
>  static inline void add_rchar(struct task_struct *tsk, ssize_t amt)
>  {
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c
> +++ linux-2.6/kernel/sched.c
> @@ -9466,6 +9466,16 @@ static int sched_rt_global_constraints(v
>  
>  	return ret;
>  }
> +
> +int sched_rt_can_attach(struct task_group *tg, struct task_struct *tsk)
> +{
> +	/* Don't accept realtime tasks when there is no way for them to run */
> +	if (rt_task(tsk) && tg->rt_bandwidth.rt_runtime == 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  #else /* !CONFIG_RT_GROUP_SCHED */
>  static int sched_rt_global_constraints(void)
>  {
> @@ -9559,8 +9569,7 @@ cpu_cgroup_can_attach(struct cgroup_subs
>  		      struct task_struct *tsk)
>  {
>  #ifdef CONFIG_RT_GROUP_SCHED
> -	/* Don't accept realtime tasks when there is no way for them to run */
> -	if (rt_task(tsk) && cgroup_tg(cgrp)->rt_bandwidth.rt_runtime == 0)
> +	if (sched_rt_can_attach(cgroup_tg(cgrp), tsk))
>  		return -EINVAL;
>  #else
>  	/* We don't support RT-tasks being in separate groups */
> Index: linux-2.6/kernel/user.c
> ===================================================================
> --- linux-2.6.orig/kernel/user.c
> +++ linux-2.6/kernel/user.c
> @@ -216,8 +216,28 @@ static ssize_t cpu_rt_period_store(struc
>  
>  static struct kobj_attribute cpu_rt_period_attr =
>  	__ATTR(cpu_rt_period, 0644, cpu_rt_period_show, cpu_rt_period_store);
> +
>  #endif
>  
> +#ifdef CONFIG_RT_GROUP_SCHED && CONFIG_USER_SCHED
> +/*
> + * We need to check if a setuid can take place. This function should be called
> + * before successfully completing the setuid.
> + */
> +
> +int rt_task_can_switch_user(uid_t uid, struct task_struct *tsk)
> +{
> +	struct user_struct *up = find_user(uid);
> +
> +	return sched_rt_can_attach(up->tg, tsk);
> +}

Should that not be !sched_rt_can_attach(), or preferably change
sched_rt_can_attach() to return a boolean, like the name implies.

> +#else
> +int rt_task_can_switch_user(uid_t uid, struct task_struct *tsk)
> +{
> +	return 0;
> +}

With the below this means !RT_GROUP_SCHED || CGROUP never gets to change
uid :-)

> +#endif
>  /* default attributes per uid directory */
>  static struct attribute *uids_attributes[] = {
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> Index: linux-2.6/kernel/sys.c
> ===================================================================
> --- linux-2.6.orig/kernel/sys.c
> +++ linux-2.6/kernel/sys.c
> @@ -579,6 +579,15 @@ static int set_user(struct cred *new)
>  		return -EAGAIN;
>  	}
>  
> +	/*
> +	 * Though rt_task_can_switch_user returns EINVAL on failure, we
> +	 * return EAGAIN so as not to break semantics and because
> +	 * EAGAIN implies resource not available which is the case in
> +	 * this case.
> +	 */
> +	if (rt_task_can_switch_user(new->uid, current))
> +		return -EAGAIN;
> +
>  	free_uid(new->user);
>  	new->user = new_user;
>  	return 0;
> 

--
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/