diff options
| author | Alban Bedel <albeu@free.fr> | 2020-04-23 06:35:23 +0300 |
|---|---|---|
| committer | Petr Štetiar <ynezz@true.cz> | 2020-05-21 16:58:46 +0300 |
| commit | 89fb6136ad7484e4e8f9b618e530e098cf573665 (patch) | |
| tree | dfdeba8aae0fbb603125d3ab9b5b93434d0375fe /tests/cram | |
| parent | 1db3e7df31d9f0ab24bcaa3fd17e81a9f3104615 (diff) | |
libubox: runqueue: fix use-after-free bug
Fixes a use-after-free bug in runqueue_task_kill():
Invalid read of size 8
at runqueue_task_kill (runqueue.c:200)
by uloop_process_timeouts (uloop.c:505)
by uloop_run_timeout (uloop.c:542)
by uloop_run (uloop.h:111)
by main (tests/test-runqueue.c:126)
Address 0x5a4b058 is 24 bytes inside a block of size 208 free'd
at free
by runqueue_task_complete (runqueue.c:234)
by runqueue_task_kill (runqueue.c:199)
by uloop_process_timeouts (uloop.c:505)
by uloop_run_timeout (uloop.c:542)
by uloop_run (uloop.h:111)
by main (tests/test-runqueue.c:126)
Block was alloc'd at
at calloc
by add_sleeper (tests/test-runqueue.c:101)
by main (tests/test-runqueue.c:123)
Since commit 11e8afea (runqueue should call 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 frees 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().
The issue was that the _complete() callback frees the memory used by the
task struct, which is then read after the _complete() callback returns.
Ref: FS#3016
Signed-off-by: Alban Bedel <albeu@free.fr>
[initial test case, kill cb comment fix]
Signed-off-by: Chris Nisbet <nischris@gmail.com>
[testcase improvements and commit subject/description tweaks]
Signed-off-by: Petr Štetiar <ynezz@true.cz>
Diffstat (limited to 'tests/cram')
| -rw-r--r-- | tests/cram/test_runqueue.t | 46 |
1 files changed, 28 insertions, 18 deletions
diff --git a/tests/cram/test_runqueue.t b/tests/cram/test_runqueue.t index 227f414..cd9d96b 100644 --- a/tests/cram/test_runqueue.t +++ b/tests/cram/test_runqueue.t @@ -2,25 +2,35 @@ check that runqueue is producing expected results: $ [ -n "$TEST_BIN_DIR" ] && export PATH="$TEST_BIN_DIR:$PATH" $ valgrind --quiet --leak-check=full test-runqueue - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' + [1/1] start 'sleep 1' (killer) + [1/1] killing process (killer) + [0/1] finish 'sleep 1' (killer) + [0/1] finish 'sleep 1' (killer) + [0/1] finish 'sleep 1' (killer) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) All done! $ test-runqueue-san - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' - [1/1] start 'sleep 1' - [1/1] cancel 'sleep 1' - [0/1] finish 'sleep 1' + [1/1] start 'sleep 1' (killer) + [1/1] killing process (killer) + [0/1] finish 'sleep 1' (killer) + [0/1] finish 'sleep 1' (killer) + [0/1] finish 'sleep 1' (killer) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) + [1/1] start 'sleep 1' (sleeper) + [1/1] cancel 'sleep 1' (sleeper) + [0/1] finish 'sleep 1' (sleeper) All done! |