Commit 4ecd9542 authored by Matthew Wilcox's avatar Matthew Wilcox

ida: Free correct IDA bitmap

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: default avatarDmitry Vyukov <dvyukov@google.com>
Signed-off-by: default avatarMatthew Wilcox <mawilcox@microsoft.com>
parent 3f1b6f9d
...@@ -2129,7 +2129,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp) ...@@ -2129,7 +2129,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp); struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
if (!bitmap) if (!bitmap)
return 0; return 0;
bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap); if (this_cpu_cmpxchg(ida_bitmap, NULL, bitmap))
kfree(bitmap); kfree(bitmap);
} }
......
...@@ -363,7 +363,7 @@ void ida_check_random(void) ...@@ -363,7 +363,7 @@ void ida_check_random(void)
{ {
DEFINE_IDA(ida); DEFINE_IDA(ida);
DECLARE_BITMAP(bitmap, 2048); DECLARE_BITMAP(bitmap, 2048);
int id; int id, err;
unsigned int i; unsigned int i;
time_t s = time(NULL); time_t s = time(NULL);
...@@ -377,8 +377,11 @@ void ida_check_random(void) ...@@ -377,8 +377,11 @@ void ida_check_random(void)
ida_remove(&ida, bit); ida_remove(&ida, bit);
} else { } else {
__set_bit(bit, bitmap); __set_bit(bit, bitmap);
do {
ida_pre_get(&ida, GFP_KERNEL); ida_pre_get(&ida, GFP_KERNEL);
assert(!ida_get_new_above(&ida, bit, &id)); err = ida_get_new_above(&ida, bit, &id);
} while (err == -ENOMEM);
assert(!err);
assert(id == bit); assert(id == bit);
} }
} }
...@@ -476,11 +479,36 @@ void ida_checks(void) ...@@ -476,11 +479,36 @@ void ida_checks(void)
radix_tree_cpu_dead(1); 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) int __weak main(void)
{ {
radix_tree_init(); radix_tree_init();
idr_checks(); idr_checks();
ida_checks(); ida_checks();
ida_thread_tests();
radix_tree_cpu_dead(1);
rcu_barrier(); rcu_barrier();
if (nr_allocated) if (nr_allocated)
printf("nr_allocated = %d\n", nr_allocated); printf("nr_allocated = %d\n", nr_allocated);
......
...@@ -368,6 +368,7 @@ int main(int argc, char **argv) ...@@ -368,6 +368,7 @@ int main(int argc, char **argv)
iteration_test(0, 10 + 90 * long_run); iteration_test(0, 10 + 90 * long_run);
iteration_test(7, 10 + 90 * long_run); iteration_test(7, 10 + 90 * long_run);
single_thread_tests(long_run); single_thread_tests(long_run);
ida_thread_tests();
/* Free any remaining preallocated nodes */ /* Free any remaining preallocated nodes */
radix_tree_cpu_dead(0); radix_tree_cpu_dead(0);
......
...@@ -36,6 +36,7 @@ void iteration_test(unsigned order, unsigned duration); ...@@ -36,6 +36,7 @@ void iteration_test(unsigned order, unsigned duration);
void benchmark(void); void benchmark(void);
void idr_checks(void); void idr_checks(void);
void ida_checks(void); void ida_checks(void);
void ida_thread_tests(void);
struct item * struct item *
item_tag_set(struct radix_tree_root *root, unsigned long index, int tag); item_tag_set(struct radix_tree_root *root, unsigned long index, int tag);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment