bcachefs: Fix oops in btree_node_seq_matches()

btree_update_nodes_written() needs to wait on in-flight writes to old
nodes before marking them as freed. But it has no reason to pin those
old nodes in memory, so some trickyness ensues.

The update we're completing deleted references to those nodes from the
btree, so we know if they've been evicted they can't be pulled back in.
We just have to check if the nodes we have pointers to are still those
old nodes, and haven't been reused.

To do that we check the node's "sequence number" (actually a random 64
bit cookie), but that lives in the node's data buffer. 'struct btree'
can't be freed until filesystem shutdown (as they're quite small), but
the data buffers can be freed or swapped around.

Commit 1f88c3567495, which was fixing a kmsan warning, assumed that we
could safely do this locklessly with just a READ_ONCE() - if we've got a
non-null ptr it would be safe to read from.

But that's not true if the data buffer is a vmalloc allocation, so we
need to restore the locking that commit deleted (or alternatively RCU
free those data buffers, but there's no other reason for that).

Fixes: 1f88c3567495 ("bcachefs: Fix a KMSAN splat in btree_update_nodes_written()")
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 e363900..d2ecb78 100644 (file)
--- a/fs/bcachefs/btree_update_interior.c
+++ b/fs/bcachefs/btree_update_interior.c
@@ -684,12 +684,31 @@ static void btree_update_nodes_written(struct btree_update *as)
 
        /*
         * Wait for any in flight writes to finish before we free the old nodes
-        * on disk:
+        * on disk. But we haven't pinned those old nodes in the btree cache,
+        * they might have already been evicted.
+        *
+        * The update we're completing deleted references to those nodes from the
+        * btree, so we know if they've been evicted they can't be pulled back in.
+        * We just have to check if the nodes we have pointers to are still those
+        * old nodes, and haven't been reused.
+        *
+        * This can't be done locklessly because the data buffer might have been
+        * vmalloc allocated, and they're not RCU freed. We also need the
+        * __no_kmsan_checks annotation because even with the btree node read
+        * lock, nothing tells us that the data buffer has been initialized (if
+        * the btree node has been reused for a different node, and the data
+        * buffer swapped for a new data buffer).
         */
        for (i = 0; i < as->nr_old_nodes; i++) {
                b = as->old_nodes[i];
 
-               if (btree_node_seq_matches(b, as->old_nodes_seq[i]))
+               bch2_trans_begin(trans);
+               btree_node_lock_nopath_nofail(trans, &b->c, SIX_LOCK_read);
+               bool seq_matches = btree_node_seq_matches(b, as->old_nodes_seq[i]);
+               six_unlock_read(&b->c.lock);
+               bch2_trans_unlock_long(trans);
+
+               if (seq_matches)
                        wait_on_bit_io(&b->flags, BTREE_NODE_write_in_flight_inner,
                                       TASK_UNINTERRUPTIBLE);
        }