On Mon, Oct 18, 2004 at 12:14:32PM -0700, Chris Wright wrote:
>
> > JJ, you wanna share your patch with the LSM list?
>
> That would be nice. Why wasn't that done in the first place?
>
okay, now that I am on the lsm list, I can share the patch and test
program directly.
The bug occurs when either of the lsm hooks capable or
security_task_setscheduler calls anything that could lock the
run queue (for us this was printk). Since the dead lock
happen on the run queue spinlock it will only manifest it self
on preempt or SMP kernels. Its a guarenteed dead lock for SMP
kernels on UP machines.
The specific conditions for printk to cause the dead lock are:
klogd must be running,
klogd must be quiescent (that is waiting sleeping, waiting for a
call to printk to wake it.)
If klogd holds the console lock (it currently dumping data) or is
not running the dead lock won't happen.
You can find a simple testmodule and test program at
http://hermes.wirex.com/~jj/
The patch to setscheduler moves the locking of the runqueue, until after
the calls to capable and security_task_setscheduler, so it fixes both of
them. I believe this to be safe, but it really needs vetting by a
scheduler person.
jj
--- kernel/sched.c.JJ 2004-07-01 08:59:55.000000000 -0700
+++ kernel/sched.c 2004-10-08 19:18:02.685437432 -0700
@@ -2682,19 +2682,13 @@
if (!p)
goto out_unlock_tasklist;
- /*
- * To be able to change p->policy safely, the apropriate
- * runqueue lock must be held.
- */
- rq = task_rq_lock(p, &flags);
-
if (policy < 0)
policy = p->policy;
else {
retval = -EINVAL;
if (policy != SCHED_FIFO && policy != SCHED_RR &&
policy != SCHED_NORMAL)
- goto out_unlock;
+ goto
- out_unlock_tasklist;
}
/*
@@ -2703,21 +2697,27 @@
*/
retval = -EINVAL;
if (lp.sched_priority < 0 || lp.sched_priority > MAX_USER_RT_PRIO-1)
- goto out_unlock;
+ goto out_unlock_tasklist;
if ((policy == SCHED_NORMAL) != (lp.sched_priority == 0))
- goto out_unlock;
+ goto out_unlock_tasklist;
retval = -EPERM;
if ((policy == SCHED_FIFO || policy == SCHED_RR) &&
!capable(CAP_SYS_NICE))
- goto out_unlock;
+ goto out_unlock_tasklist;
if ((current->euid != p->euid) && (current->euid !=
p->uid) &&
!capable(CAP_SYS_NICE))
- goto out_unlock;
+ goto out_unlock_tasklist;
retval = security_task_setscheduler(p, policy, &lp);
if (retval)
- goto out_unlock;
+ goto out_unlock_tasklist;
+
+ /*
+ * To be able to change p->policy safely, the apropriate
+ * runqueue lock must be held.
+ */
+ rq = task_rq_lock(p, &flags);
array = p->array;
if (array)
@@ -2739,7 +2739,6 @@
resched_task(rq->curr);
}
-out_unlock:
task_rq_unlock(rq, &flags);
out_unlock_tasklist:
read_unlock_irq(&tasklist_lock);
This archive was generated by hypermail 2.1.3 : Mon Oct 18 2004 - 14:53:19 PDT