[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 1d7a420c84)
This commit is contained in:
SaVaGe 2009-10-06 18:53:37 +03:00 committed by Willy Tarreau
parent 59f4a5bd64
commit 22c0257059
1 changed files with 3 additions and 1 deletions

View File

@ -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;
}