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

From: Dhaval Giani
Date: Mon Feb 16 2009 - 08:20:55 EST


On Mon, Feb 16, 2009 at 06:44:40PM +0530, Dhaval Giani wrote:
> On Mon, Feb 16, 2009 at 01:24:42PM +0100, Peter Zijlstra wrote:
> > On Mon, 2009-02-16 at 17:32 +0530, Dhaval Giani wrote:
> >
> > > Yeah, made it boolean. how does the following look?
> >
> > Much better, but look below.
> >
>
> <snip>
>
> > > 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;
> >
> > you're leaking new_user here.
> >
> > Best might be to place this test on top before allocating it.
> >
>
> Yep, also another memory leak was there, which I fixed in this version.
> What about this? (This is not a good day!)
>

And it continues on! Please try this version.

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 v3:
1. Actually fix the leak.

Changes from v2:
1. Patch compiles for CONFIG_CGROUP_SCHED as well
2. Fix two memory leaks.

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@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);
+extern int sched_rt_can_attach(struct task_group *tg, struct task_struct *tsk);
#endif
#endif

+extern 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
@@ -362,6 +362,28 @@ static void free_user(struct user_struct

#endif

+#if defined(CONFIG_RT_GROUP_SCHED) && defined(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)
+{
+ int ret = 1;
+ struct user_struct *up = find_user(uid);
+
+ ret = sched_rt_can_attach(up->tg, tsk);
+ free_uid(up);
+
+ return ret;
+}
+#else
+int task_can_switch_user(uid_t uid, struct task_struct *tsk)
+{
+ return 1;
+}
+#endif
+
/*
* Locate the user_struct for the passed UID. If found, take a ref on it. The
* caller must undo that ref with free_uid().
Index: linux-2.6/kernel/sys.c
===================================================================
--- linux-2.6.orig/kernel/sys.c
+++ linux-2.6/kernel/sys.c
@@ -560,7 +560,7 @@ error:
abort_creds(new);
return retval;
}
-
+
/*
* change the user struct in a credentials set to match the new UID
*/
@@ -568,6 +568,9 @@ static int set_user(struct cred *new)
{
struct user_struct *new_user;

+ if (!task_can_switch_user(new->uid, current))
+ return -EAGAIN;
+
new_user = alloc_uid(current_user_ns(), new->uid);
if (!new_user)
return -EAGAIN;
--
regards,
Dhaval
--
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/