radix tree: Don't return retry entries from lookup

Commit 66ee620f06f9 ("idr: Permit any valid kernel pointer to be stored")
changed the radix tree lookup so that it stops when reaching the bottom
of the tree.  However, the condition was added in the wrong place,
making it possible to return retry entries to the caller.  Reorder the
tests to check for the retry entry before checking whether we're at the
bottom of the tree.  The retry entry should never be found in the tree
root, so it's safe to defer the check until the end of the loop.

Add a regression test to the test-suite to be sure this doesn't come
back.

Fixes: 66ee620f06f9 ("idr: Permit any valid kernel pointer to be stored")
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Matthew Wilcox <willy@infradead.org>

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index 1106bb6aa01e977de26bcb1235080b00b0e4a067..14d51548bea6414f9b256dd0179830d07dce1581 100644 (file)
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -784,11 +784,11 @@ void *__radix_tree_lookup(const struct radix_tree_root *root,
        while (radix_tree_is_internal_node(node)) {
                unsigned offset;
 
-               if (node == RADIX_TREE_RETRY)
-                       goto restart;
                parent = entry_to_node(node);
                offset = radix_tree_descend(parent, &node, index);
                slot = parent->slots + offset;
+               if (node == RADIX_TREE_RETRY)
+                       goto restart;
                if (parent->shift == 0)
                        break;
        }

diff --git a/tools/testing/radix-tree/Makefile b/tools/testing/radix-tree/Makefile
index acf1afa01c5b9ce26e55655e8957ae14794e9c83..397d6b612502de8c13b6f4862e5b642d25f37ee1 100644 (file)
--- a/tools/testing/radix-tree/Makefile
+++ b/tools/testing/radix-tree/Makefile
@@ -7,6 +7,7 @@ LDLIBS+= -lpthread -lurcu
 TARGETS = main idr-test multiorder xarray
 CORE_OFILES := xarray.o radix-tree.o idr.o linux.o test.o find_bit.o bitmap.o
 OFILES = main.o $(CORE_OFILES) regression1.o regression2.o regression3.o \
+        regression4.o \
         tag_check.o multiorder.o idr-test.o iteration_check.o benchmark.o
 
 ifndef SHIFT

diff --git a/tools/testing/radix-tree/main.c b/tools/testing/radix-tree/main.c
index 77a44c54998f0ef908a80e4540c188b02afe18b4..7a22d6e3732e5be22a3dfce09ee0071dff0babc8 100644 (file)
--- a/tools/testing/radix-tree/main.c
+++ b/tools/testing/radix-tree/main.c
@@ -308,6 +308,7 @@ int main(int argc, char **argv)
        regression1_test();
        regression2_test();
        regression3_test();
+       regression4_test();
        iteration_test(0, 10 + 90 * long_run);
        iteration_test(7, 10 + 90 * long_run);
        single_thread_tests(long_run);

diff --git a/tools/testing/radix-tree/regression.h b/tools/testing/radix-tree/regression.h
index 3c8a1584e9ee4c84be01b3ecda5cfbcbccd24fd3..135145af18b7c17b7943b89139506ab142702aad 100644 (file)
--- a/tools/testing/radix-tree/regression.h
+++ b/tools/testing/radix-tree/regression.h
@@ -5,5 +5,6 @@
 void regression1_test(void);
 void regression2_test(void);
 void regression3_test(void);
+void regression4_test(void);
 
 #endif

diff --git a/tools/testing/radix-tree/regression4.c b/tools/testing/radix-tree/regression4.c
new file mode 100644 (file)
index 0000000..cf4e5ab
--- /dev/null
+++ b/tools/testing/radix-tree/regression4.c
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/kernel.h>
+#include <linux/gfp.h>
+#include <linux/slab.h>
+#include <linux/radix-tree.h>
+#include <linux/rcupdate.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <assert.h>
+
+#include "regression.h"
+
+static pthread_barrier_t worker_barrier;
+static int obj0, obj1;
+static RADIX_TREE(mt_tree, GFP_KERNEL);
+
+static void *reader_fn(void *arg)
+{
+       int i;
+       void *entry;
+
+       rcu_register_thread();
+       pthread_barrier_wait(&worker_barrier);
+
+       for (i = 0; i < 1000000; i++) {
+               rcu_read_lock();
+               entry = radix_tree_lookup(&mt_tree, 0);
+               rcu_read_unlock();
+               if (entry != &obj0) {
+                       printf("iteration %d bad entry = %p\n", i, entry);
+                       abort();
+               }
+       }
+
+       rcu_unregister_thread();
+
+       return NULL;
+}
+
+static void *writer_fn(void *arg)
+{
+       int i;
+
+       rcu_register_thread();
+       pthread_barrier_wait(&worker_barrier);
+
+       for (i = 0; i < 1000000; i++) {
+               radix_tree_insert(&mt_tree, 1, &obj1);
+               radix_tree_delete(&mt_tree, 1);
+       }
+
+       rcu_unregister_thread();
+
+       return NULL;
+}
+
+void regression4_test(void)
+{
+       pthread_t reader, writer;
+
+       printv(1, "regression test 4 starting\n");
+
+       radix_tree_insert(&mt_tree, 0, &obj0);
+       pthread_barrier_init(&worker_barrier, NULL, 2);
+
+       if (pthread_create(&reader, NULL, reader_fn, NULL) ||
+           pthread_create(&writer, NULL, writer_fn, NULL)) {
+               perror("pthread_create");
+               exit(1);
+       }
+
+       if (pthread_join(reader, NULL) || pthread_join(writer, NULL)) {
+               perror("pthread_join");
+               exit(1);
+       }
+
+       printv(1, "regression test 4 passed\n");
+}