lkml.org 
[lkml]   [2009]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch 2/2] sched: Protect sched_rr_get_param access to task->sched_class
On Wed, 9 Dec 2009, Peter Zijlstra wrote:

> On Wed, 2009-12-09 at 09:00 +0100, Peter Zijlstra wrote:
> > On Tue, 2009-12-08 at 20:24 +0000, Thomas Gleixner wrote:
> > > plain text document attachment
> > > (sched-fix-sched-get-rr-param-race.patch)
> > > sched_rr_get_param calls task->sched_class->get_rr_interval(task)
> > > without protection against a concurrent sched_setscheduler() call
> > > which modifies task->sched_class.
> > >
> > > Serialize the access with task_rq_lock(task);
> > >
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > > ---
> > > kernel/sched.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > Index: linux-2.6-tip/kernel/sched.c
> > > ===================================================================
> > > --- linux-2.6-tip.orig/kernel/sched.c
> > > +++ linux-2.6-tip/kernel/sched.c
> > > @@ -6887,6 +6887,8 @@ SYSCALL_DEFINE2(sched_rr_get_interval, p
> > > {
> > > struct task_struct *p;
> > > unsigned int time_slice;
> > > + unsigned long flags;
> > > + struct rq *rq;
> > > int retval;
> > > struct timespec t;
> > >
> > > @@ -6903,7 +6905,9 @@ SYSCALL_DEFINE2(sched_rr_get_interval, p
> > > if (retval)
> > > goto out_unlock;
> > >
> > > + rq = task_rq_lock(p, &flags);
> > > time_slice = p->sched_class->get_rr_interval(p);
> > > + task_rq_unlock(rq, &flags);
> > >
> > > read_unlock(&tasklist_lock);
> > > jiffies_to_timespec(time_slice, &t);
> > >
> >
> > Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
>
> Oh, that was too quick, get_rr_interval_fair() actually takes the
> rq->lock, so lets add this:
>
> unsigned int get_rr_interval_fair(struct task_struct *task)
> {
> struct sched_entity *se = &task->se;
> - unsigned long flags;
> - struct rq *rq;
> unsigned int rr_interval = 0;
>
> /*
> * Time slice is 0 for SCHED_OTHER tasks that are on an otherwise
> * idle runqueue:
> */
> - rq = task_rq_lock(task, &flags);
> if (rq->cfs.load.weight)
> rr_interval = NS_TO_JIFFIES(sched_slice(&rq->cfs, se));
> - task_rq_unlock(rq, &flags);
> -
> return rr_interval;
> }

Duh, yes. I looked at that actually and made a mental note to remove it. :(

Thanks for catching that,

tglx


\
 
 \ /
  Last update: 2009-12-09 09:09    [W:0.033 / U:0.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site