| Message ID | 20200824081353.25148-1-walter-zh.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Mon, 24 Aug 2020 at 10:14, Walter Wu <walter-zh.wu@mediatek.com> wrote: > > Adds a test to verify workqueue stack recording and print it in > KASAN report. > > The KASAN report was as follows(cleaned up slightly): > > BUG: KASAN: use-after-free in kasan_workqueue_uaf > > Freed by task 54: > kasan_save_stack+0x24/0x50 > kasan_set_track+0x24/0x38 > kasan_set_free_info+0x20/0x40 > __kasan_slab_free+0x10c/0x170 > kasan_slab_free+0x10/0x18 > kfree+0x98/0x270 > kasan_workqueue_work+0xc/0x18 > > Last potentially related work creation: > kasan_save_stack+0x24/0x50 > kasan_record_wq_stack+0xa8/0xb8 > insert_work+0x48/0x288 > __queue_work+0x3e8/0xc40 > queue_work_on+0xf4/0x118 > kasan_workqueue_uaf+0xfc/0x190 > > Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com> > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Matthias Brugger <matthias.bgg@gmail.com> > Cc: Andrey Konovalov <andreyknvl@google.com> > --- > lib/test_kasan.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) These will majorly conflict with the KASAN-test KUnit rework, which I don't know what the status is. AFAIK, these are not yet in -mm tree. I think the KASAN-test KUnit rework has priority, as rebasing that work on top of this patch is going to be difficult. So maybe these test additions can be declared optional if there are conflicts coming, and if that'll be the case you'll have to rebase and resend the test. Thanks, -- Marco
On Mon, Aug 24, 2020 at 1:49 PM Marco Elver <elver@google.com> wrote: > > On Mon, 24 Aug 2020 at 10:14, Walter Wu <walter-zh.wu@mediatek.com> wrote: > > > > Adds a test to verify workqueue stack recording and print it in > > KASAN report. > > > > The KASAN report was as follows(cleaned up slightly): > > > > BUG: KASAN: use-after-free in kasan_workqueue_uaf > > > > Freed by task 54: > > kasan_save_stack+0x24/0x50 > > kasan_set_track+0x24/0x38 > > kasan_set_free_info+0x20/0x40 > > __kasan_slab_free+0x10c/0x170 > > kasan_slab_free+0x10/0x18 > > kfree+0x98/0x270 > > kasan_workqueue_work+0xc/0x18 > > > > Last potentially related work creation: > > kasan_save_stack+0x24/0x50 > > kasan_record_wq_stack+0xa8/0xb8 > > insert_work+0x48/0x288 > > __queue_work+0x3e8/0xc40 > > queue_work_on+0xf4/0x118 > > kasan_workqueue_uaf+0xfc/0x190 > > > > Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com> > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Cc: Alexander Potapenko <glider@google.com> > > Cc: Matthias Brugger <matthias.bgg@gmail.com> > > Cc: Andrey Konovalov <andreyknvl@google.com> > > --- > > lib/test_kasan.c | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > These will majorly conflict with the KASAN-test KUnit rework, which I > don't know what the status is. AFAIK, these are not yet in -mm tree. I've asked Andrew to take those in 5.9, but that didn't happen. Perhaps we should ping him again after Plumbers. > I think the KASAN-test KUnit rework has priority, as rebasing that > work on top of this patch is going to be difficult. So maybe these > test additions can be declared optional if there are conflicts coming, > and if that'll be the case you'll have to rebase and resend the test. Yeah, either waiting for KASAN+Kunit or separating the tests sounds like plausible approaches.
diff --git a/lib/test_kasan.c b/lib/test_kasan.c index 2bd61674c7a3..7293a55ff51c 100644 --- a/lib/test_kasan.c +++ b/lib/test_kasan.c @@ -845,6 +845,34 @@ static noinline void __init kasan_timer_uaf(void) ((volatile struct timer_list *)timer)->expires; } +static noinline void __init kasan_workqueue_work(struct work_struct *work) +{ + kfree(work); +} + +static noinline void __init kasan_workqueue_uaf(void) +{ + struct workqueue_struct *workqueue; + struct work_struct *work; + + workqueue = create_workqueue("kasan_wq_test"); + if (!workqueue) { + pr_err("Allocation failed\n"); + return; + } + work = kmalloc(sizeof(struct work_struct), GFP_KERNEL); + if (!work) { + pr_err("Allocation failed\n"); + return; + } + + INIT_WORK(work, kasan_workqueue_work); + queue_work(workqueue, work); + destroy_workqueue(workqueue); + + pr_info("use-after-free on workqueue\n"); + ((volatile struct work_struct *)work)->data; +} static int __init kmalloc_tests_init(void) { /* @@ -894,6 +922,7 @@ static int __init kmalloc_tests_init(void) vmalloc_oob(); kasan_rcu_uaf(); kasan_timer_uaf(); + kasan_workqueue_uaf(); kasan_restore_multi_shot(multishot);
Adds a test to verify workqueue stack recording and print it in KASAN report. The KASAN report was as follows(cleaned up slightly): BUG: KASAN: use-after-free in kasan_workqueue_uaf Freed by task 54: kasan_save_stack+0x24/0x50 kasan_set_track+0x24/0x38 kasan_set_free_info+0x20/0x40 __kasan_slab_free+0x10c/0x170 kasan_slab_free+0x10/0x18 kfree+0x98/0x270 kasan_workqueue_work+0xc/0x18 Last potentially related work creation: kasan_save_stack+0x24/0x50 kasan_record_wq_stack+0xa8/0xb8 insert_work+0x48/0x288 __queue_work+0x3e8/0xc40 queue_work_on+0xf4/0x118 kasan_workqueue_uaf+0xfc/0x190 Signed-off-by: Walter Wu <walter-zh.wu@mediatek.com> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Alexander Potapenko <glider@google.com> Cc: Matthias Brugger <matthias.bgg@gmail.com> Cc: Andrey Konovalov <andreyknvl@google.com> --- lib/test_kasan.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)