[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090216120213.GB3925@linux.vnet.ibm.com>
Date: Mon, 16 Feb 2009 17:32:13 +0530
From: Dhaval Giani <dhaval@...ux.vnet.ibm.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Corey Hickey <bugfood-ml@...ooh.org>, linux-kernel@...r.kernel.org,
Bharata B Rao <bharata@...ux.vnet.ibm.com>,
Balbir Singh <balbir@...ibm.com>,
Srivatsa Vaddagiri <vatsa@...ux.vnet.ibm.com>,
Ingo Molnar <mingo@...e.hu>, mtk.manpages@...il.com
Subject: Re: RT scheduling and a way to make a process hang, unkillable
On Mon, Feb 16, 2009 at 12:08:36PM +0100, Peter Zijlstra wrote:
> 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@...ux.vnet.ibm.com>
> >
> > 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.
>
Yeah, made it boolean. how does the following look?
--
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.
Changes from v1:
1. Peter suggested that rt_task_can_change_user should be renamed to
task_can_change_user
2. Changed sched_rt_can_attach to boolean.
Signed-off-by: Dhaval Giani <dhaval@...ux.vnet.ibm.com>
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 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 0;
+
+ return 1;
+}
+
#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 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);
+}
+#else
+int task_can_switch_user(uid_t uid, struct task_struct *tsk)
+{
+ return 1;
+}
+
+#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,9 @@ static int set_user(struct cred *new)
return -EAGAIN;
}
+ if (!task_can_switch_user(new->uid, current))
+ return -EAGAIN;
+
free_uid(new->user);
new->user = new_user;
return 0;
--
regards,
Dhaval
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists