From 22c02570596ddb8c1f2f3195763c0a87ba7a4998 Mon Sep 17 00:00:00 2001 From: SaVaGe Date: Tue, 6 Oct 2009 18:53:37 +0300 Subject: [PATCH] [BUG] task.c: don't assing last_timer to node-less entries I noticed that in __eb32_insert , if the tree is empty (root->b[EB_LEFT] == NULL) , the node.bit is not defined. However in __task_queue there are checks: - if (last_timer->node.bit < 0) - if (task->wq.node.bit < last_timer->node.bit) which might rely upon an undefined value. This is how I see it: 1. We insert eb32_node in an empty wait queue tree for a task (called by process_runnable_tasks() ): Inserting into empty wait queue &task->wq = 0x72a87c8, last_timer pointer: (nil) 2. Then, we set the last timer to the same address: Setting last_timer: (nil) to: 0x72a87c8 3. We get a new task to be inserted in the queue (again called by process_runnable_tasks()) , before the __task_unlink_wq() is called for the previous task. 4. At this point, we still have last_timer set to 0x72a87c8 , but since it was inserted in an empty tree, it doesn't have node.bit and the values above get dereferenced with undefined value. The bug has no effect right now because the check for equality is still made, so the next timer will still be queued at the right place anyway, without any possible side-effect. But it's a pending bug waiting for a small change somewhere to strike. Iliya Polihronov (cherry picked from commit 1d7a420c84cfd19bfeaedfc1dc971fb13dfc8a1f) --- src/task.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/task.c b/src/task.c index 268f8d9e97..cc455f58fe 100644 --- a/src/task.c +++ b/src/task.c @@ -108,7 +108,9 @@ void __task_queue(struct task *task) return; } eb32_insert(&timers, &task->wq); - if (!last_timer || (task->wq.node.bit < last_timer->node.bit)) + + /* Make sure we don't assign the last_timer to a node-less entry */ + if (task->wq.node.node_p && (!last_timer || (task->wq.node.bit < last_timer->node.bit))) last_timer = &task->wq; return; }