maple_tree: Fix memory leak when two threads race

If another thread succeeds in allocating the node while we're sleeping
to allocate memory, we'll never use the ms->alloc, so it has to be freed
by mas_nomem().

Signed-off-by: Matthew Wilcox <willy@infradead.org>

diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
index d42cb5da74be9f095aa7caca9e372864722adee4..7ba0432c8c4019d120abcee9d0ae413564994f46 100644 (file)
--- a/include/linux/maple_tree.h
+++ b/include/linux/maple_tree.h
@@ -150,6 +150,7 @@ struct maple_state {
  */
 #define MAS_START      ((struct maple_node *)1UL)
 #define MAS_ROOT       ((struct maple_node *)3UL)
+#define MA_ERROR(err)  ((struct maple_node *)(((unsigned long)err << 2) | 2UL))
 
 #define MAP_STATE(sname, stree, sindex, eindex)\
        struct maple_state sname = {                            \
@@ -193,4 +194,7 @@ static inline bool mtree_empty(const struct maple_tree *mt)
        return mt->root == NULL;
 }
 
+/* Advanced API */
+
+bool mas_nomem(struct maple_state *, gfp_t);
 #endif

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 6fd0ee8d3ef66af7d10992d6aac27f0f38083112..a1f0c041add7a4ddd232b005e5bb38375c15ea04 100644 (file)
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -6,8 +6,6 @@
 #include <linux/maple_tree.h>
 #include <linux/rcupdate.h>
 
-#define MA_ERROR(err) ((struct maple_node *)(((unsigned long)err << 2) | 2UL))
-
 static inline void mas_set_err(struct maple_state *mas, long err)
 {
        mas->node = MA_ERROR(err);
@@ -155,11 +153,19 @@ list_failed:
        mas_set_err(ms, -ENOMEM);
 }
 
-static bool __maple_nomem(struct maple_state *ms, gfp_t gfp)
+bool mas_nomem(struct maple_state *ms, gfp_t gfp)
        __must_hold(ms->tree->lock)
 {
-       if (ms->node != MA_ERROR(-ENOMEM))
+       if (ms->node != MA_ERROR(-ENOMEM)) {
+               struct maple_node *node = ma_get_alloc(ms);
+               if (node) {
+                       kfree(node->slot[1]);
+                       kfree(node->slot[0]);
+                       kfree(node);
+               }
+               ms->alloc = NULL;
                return false;
+       }
 
        if (gfpflags_allow_blocking(gfp)) {
                mtree_unlock(ms->tree);
@@ -734,7 +740,7 @@ retry:
                goto already_exists;
 
        ret = _maple_insert(&ms, entry);
-       if (__maple_nomem(&ms, gfp))
+       if (mas_nomem(&ms, gfp))
                goto retry;
 
 already_exists:

diff --git a/lib/test_maple_tree.c b/lib/test_maple_tree.c
index d7a4ec038575d3e163c29ed38dddc4d8ca5d7911..e4bf5efbc06e0208aef3f90b9bdecccaa843d60b 100644 (file)
--- a/lib/test_maple_tree.c
+++ b/lib/test_maple_tree.c
@@ -89,6 +89,8 @@ static noinline void check_load(struct maple_tree *mt, unsigned long index,
 
 static noinline void check_nomem(struct maple_tree *mt)
 {
+       MAP_STATE(ms, mt, 1, 1);
+
        MT_BUG_ON(mt, !mtree_empty(mt));
 
        /* Storing something at 1 requires memory allocation */
@@ -96,6 +98,28 @@ static noinline void check_nomem(struct maple_tree *mt)
        /* Storing something at 0 does not */
        MT_BUG_ON(mt, mtree_insert_index(mt, 0, GFP_ATOMIC) != 0);
 
+       /*
+        * Simulate two threads racing; the first one fails to allocate
+        * memory to insert an entry at 1, then the second one succeeds
+        * in allocating memory to insert an entry at 2.  The first one
+        * then needs to free the node it allocated.  LeakSanitizer will
+        * notice this, as will the 'nr_allocated' debugging aid in the
+        * userspace test suite.
+        */
+       mtree_lock(mt);
+       _maple_setup_insert(&ms);
+       MT_BUG_ON(mt, ms.node != MA_ERROR(-ENOMEM));
+       _maple_insert(&ms, xa_mk_value(1));
+       mas_nomem(&ms, GFP_KERNEL);
+       MT_BUG_ON(mt, ms.node != MAS_START);
+       mtree_unlock(mt);
+       MT_BUG_ON(mt, mtree_insert_index(mt, 2, GFP_KERNEL) != 0);
+       mtree_lock(mt);
+       _maple_setup_insert(&ms);
+       _maple_insert(&ms, xa_mk_value(1));
+       mas_nomem(&ms, GFP_KERNEL);
+       mtree_unlock(mt);
+
        mtree_destroy(mt);
 }