On 01/07/2019 16:23, Alban wrote:
On Mon, 1 Jul 2019 15:48:47 +0200
John Crispin <[email protected]> wrote:
On 21/06/2019 17:38, Alban Bedel wrote:
Since commit 11e8afea (runqueue should cal the complete handler from
more places) the call to the complete() callback has been moved to
runqueue_task_complete(). However in runqueue_task_kill()
runqueue_task_complete() is called before the kill() callback.
This will result in a use after free if the complete() callback free
the task struct.
Furthermore runqueue_start_next() is already called at the end of
runqueue_task_complete(), so there is no need to call it again in
runqueue_task_kill().
Signed-off-by: Alban Bedel <[email protected]>
---
runqueue.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/runqueue.c b/runqueue.c
index a1d0133..4c621aa 100644
--- a/runqueue.c
+++ b/runqueue.c
@@ -196,11 +196,9 @@ void runqueue_task_kill(struct runqueue_task
*t) if (!t->queued)
return;
- runqueue_task_complete(t);
if (running && t->type->kill)
t->type->kill(q, t);
-
- runqueue_start_next(q);
+ runqueue_task_complete(t);
}
void runqueue_stop(struct runqueue *q)
Hi,
runqueue_task_complete() will decrement running which, if called
after the kill clause might not even trigger the kill() call. I am
assuming you are running a custom runqueue_task_type ?
No, TBH I haven't tested this, but as there is no documentation I had
to go through the code and noticed this suspicious construct. I then
saw commit 6a7fb7d8d (runqueue: fix use-after-free bug) which confirmed
that complete() is expected to free the task struct, which with the
above code will clearly lead to an access after free.
But I don't really see what you mean, 'running' is a boolean so it
can't be decremented. Did you mean 'running_tasks'?
Alban
i did actually mean running_task, I'll have another look later on today
John
_______________________________________________
openwrt-devel mailing list
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
