ida: Free correct IDA bitmap
authorMatthew Wilcox <mawilcox@microsoft.com>
Fri, 3 Mar 2017 17:16:10 +0000 (12:16 -0500)
committerMatthew Wilcox <mawilcox@microsoft.com>
Tue, 7 Mar 2017 18:18:23 +0000 (13:18 -0500)
There's a relatively rare race where we look at the per-cpu preallocated
IDA bitmap, see it's NULL, allocate a new one, and atomically update it.
If the kmalloc() happened to sleep and we were rescheduled to a different
CPU, or an interrupt came in at the exact right time, another task
might have successfully allocated a bitmap and already deposited it.
I forgot what the semantics of cmpxchg() were and ended up freeing the
wrong bitmap leading to KASAN reporting a use-after-free.

Dmitry found the bug with syzkaller & wrote the patch.  I wrote the test
case that will reproduce the bug without his patch being applied.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
lib/radix-tree.c
tools/testing/radix-tree/idr-test.c
tools/testing/radix-tree/main.c
tools/testing/radix-tree/test.h

index 5ed506d648c4e53ee955e9c19b942fd0d666eee1..691a9ad48497b02e3b09304d6565165ef2317b16 100644 (file)
@@ -2129,8 +2129,8 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
                struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
                if (!bitmap)
                        return 0;
-               bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
-               kfree(bitmap);
+               if (this_cpu_cmpxchg(ida_bitmap, NULL, bitmap))
+                       kfree(bitmap);
        }
 
        return 1;
index 86de901fa5c6bec809332ad04df8b11cc4ff2d9a..30cd0b296f1a76847122f009c2cd0f2d5b9109f4 100644 (file)
@@ -363,7 +363,7 @@ void ida_check_random(void)
 {
        DEFINE_IDA(ida);
        DECLARE_BITMAP(bitmap, 2048);
-       int id;
+       int id, err;
        unsigned int i;
        time_t s = time(NULL);
 
@@ -377,8 +377,11 @@ void ida_check_random(void)
                        ida_remove(&ida, bit);
                } else {
                        __set_bit(bit, bitmap);
-                       ida_pre_get(&ida, GFP_KERNEL);
-                       assert(!ida_get_new_above(&ida, bit, &id));
+                       do {
+                               ida_pre_get(&ida, GFP_KERNEL);
+                               err = ida_get_new_above(&ida, bit, &id);
+                       } while (err == -ENOMEM);
+                       assert(!err);
                        assert(id == bit);
                }
        }
@@ -476,11 +479,36 @@ void ida_checks(void)
        radix_tree_cpu_dead(1);
 }
 
+static void *ida_random_fn(void *arg)
+{
+       rcu_register_thread();
+       ida_check_random();
+       rcu_unregister_thread();
+       return NULL;
+}
+
+void ida_thread_tests(void)
+{
+       pthread_t threads[10];
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(threads); i++)
+               if (pthread_create(&threads[i], NULL, ida_random_fn, NULL)) {
+                       perror("creating ida thread");
+                       exit(1);
+               }
+
+       while (i--)
+               pthread_join(threads[i], NULL);
+}
+
 int __weak main(void)
 {
        radix_tree_init();
        idr_checks();
        ida_checks();
+       ida_thread_tests();
+       radix_tree_cpu_dead(1);
        rcu_barrier();
        if (nr_allocated)
                printf("nr_allocated = %d\n", nr_allocated);
index b829127d56705747a0a74c73e8b11b8cae8ecc27..bc9a78449572f10331a8bbc35070801f307b6caa 100644 (file)
@@ -368,6 +368,7 @@ int main(int argc, char **argv)
        iteration_test(0, 10 + 90 * long_run);
        iteration_test(7, 10 + 90 * long_run);
        single_thread_tests(long_run);
+       ida_thread_tests();
 
        /* Free any remaining preallocated nodes */
        radix_tree_cpu_dead(0);
index b30e11d9d271c39ccb284019938876ae2785f7ca..0f8220cc61663ffa2a872db42c96b4e5433ed0a7 100644 (file)
@@ -36,6 +36,7 @@ void iteration_test(unsigned order, unsigned duration);
 void benchmark(void);
 void idr_checks(void);
 void ida_checks(void);
+void ida_thread_tests(void);
 
 struct item *
 item_tag_set(struct radix_tree_root *root, unsigned long index, int tag);