bcachefs: Various fixes for interior update path

The locking was wrong, and we could get a use after free in the error
path where we weren't taking the entrie being freed off the unwritten
list.

Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/btree_update_interior.c b/fs/bcachefs/btree_update_interior.c
index 68deb4eb31a69bb51ce1777aa825e7677d5eced8..3f9605f2f1f478538dcf32aaae854772ae9978e2 100644 (file)
--- a/fs/bcachefs/btree_update_interior.c
+++ b/fs/bcachefs/btree_update_interior.c
@@ -581,7 +581,7 @@ err_free:
 
 /* Asynchronous interior node update machinery */
 
-static void bch2_btree_update_free(struct btree_update *as)
+static void __bch2_btree_update_free(struct btree_update *as)
 {
        struct bch_fs *c = as->c;
 
@@ -596,28 +596,32 @@ static void bch2_btree_update_free(struct btree_update *as)
        if (as->reserve)
                bch2_btree_reserve_put(c, as->reserve);
 
-       mutex_lock(&c->btree_interior_update_lock);
        list_del(&as->list);
 
        closure_debug_destroy(&as->cl);
        mempool_free(as, &c->btree_interior_update_pool);
 
        closure_wake_up(&c->btree_interior_update_wait);
-       mutex_unlock(&c->btree_interior_update_lock);
 }
 
-static void btree_update_nodes_reachable(struct btree_update *as, u64 seq)
+static void bch2_btree_update_free(struct btree_update *as)
 {
        struct bch_fs *c = as->c;
 
        mutex_lock(&c->btree_interior_update_lock);
+       __bch2_btree_update_free(as);
+       mutex_unlock(&c->btree_interior_update_lock);
+}
+
+static void btree_update_nodes_reachable(struct btree_update *as, u64 seq)
+{
+       struct bch_fs *c = as->c;
 
        while (as->nr_new_nodes) {
                struct btree *b = as->new_nodes[--as->nr_new_nodes];
 
                BUG_ON(b->will_make_reachable != (unsigned long) as);
                b->will_make_reachable = 0;
-               mutex_unlock(&c->btree_interior_update_lock);
 
                /*
                 * b->will_make_reachable prevented it from being written, so
@@ -626,14 +630,11 @@ static void btree_update_nodes_reachable(struct btree_update *as, u64 seq)
                btree_node_lock_type(c, b, SIX_LOCK_read);
                bch2_btree_node_write_cond(c, b, btree_node_need_write(b));
                six_unlock_read(&b->c.lock);
-               mutex_lock(&c->btree_interior_update_lock);
        }
 
        while (as->nr_pending)
                bch2_btree_node_free_ondisk(c, &as->pending[--as->nr_pending],
                                            seq);
-
-       mutex_unlock(&c->btree_interior_update_lock);
 }
 
 static void btree_update_nodes_written(struct closure *cl)
@@ -667,9 +668,12 @@ again:
                mutex_unlock(&c->btree_interior_update_lock);
                btree_node_lock_type(c, b, SIX_LOCK_intent);
                six_unlock_intent(&b->c.lock);
-               goto out;
+               mutex_lock(&c->btree_interior_update_lock);
+               goto again;
        }
 
+       list_del(&as->unwritten_list);
+
        journal_u64s = 0;
 
        if (as->mode != BTREE_INTERIOR_UPDATING_ROOT)
@@ -710,9 +714,6 @@ again:
                bch2_btree_add_journal_pin(c, b, res.seq);
                six_unlock_write(&b->c.lock);
 
-               list_del(&as->unwritten_list);
-               mutex_unlock(&c->btree_interior_update_lock);
-
                /*
                 * b->write_blocked prevented it from being written, so
                 * write it now if it needs to be written:
@@ -723,9 +724,6 @@ again:
 
        case BTREE_INTERIOR_UPDATING_AS:
                BUG_ON(b);
-
-               list_del(&as->unwritten_list);
-               mutex_unlock(&c->btree_interior_update_lock);
                break;
 
        case BTREE_INTERIOR_UPDATING_ROOT: {
@@ -739,9 +737,6 @@ again:
                r->alive = true;
                c->btree_roots_dirty = true;
                mutex_unlock(&c->btree_root_lock);
-
-               list_del(&as->unwritten_list);
-               mutex_unlock(&c->btree_interior_update_lock);
                break;
        }
        }
@@ -753,14 +748,12 @@ again:
 
        btree_update_nodes_reachable(as, res.seq);
 free_update:
-       bch2_btree_update_free(as);
+       __bch2_btree_update_free(as);
        /*
         * for flush_held_btree_writes() waiting on updates to flush or
         * nodes to be writeable:
         */
        closure_wake_up(&c->btree_interior_update_wait);
-out:
-       mutex_lock(&c->btree_interior_update_lock);
        goto again;
 }