btrfs: raid56: keep a reference on a stolen cached rbio until it is dropped

lock_stripe_add() steals the pages of a cached rbio for the same full
stripe, drops the rbio's hash-list reference under the bucket lock, and
only after releasing that lock calls remove_rbio_from_cache() on it.
Between those two steps the rbio is held by nothing but the cache
reference, and anyone who removes it from the cache in that window --
the cache shrink in cache_rbio(), or btrfs_raid56_uncache_range() when
the stripe's extents are freed -- frees it.  remove_rbio_from_cache()
then dereferences a freed rbio:

  BUG: kernel NULL pointer dereference, address: 0000000000000008
  Workqueue: btrfs-rmw rmw_rbio_work
  RIP: 0010:__remove_rbio_from_cache+0x33/0x180
   remove_rbio_from_cache+0x41/0x60
   lock_stripe_add+0xfc/0x460
   rmw_rbio_work+0x35/0x400

The oops leaves the cache and bucket locks held, and every other RMW
worker and the transaction thread soft-lock behind them.  Seen after
8h of the device-corruption acceptance suite on 6.18, where freeing of
extents during read errors made the uncache path frequent; the window
exists with the plain cache shrink as well.

Keep the hash-list reference across the steal and drop it after the
cache removal, so a concurrent remover can clear the cache bit and drop
its own reference but never free the rbio under our feet.

Assisted-by: Claude:claude-opus-4-8

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index 72474825c178508a70427a1a399a9f553caddb83..deacd44a7804dcaacbdf04b78b73360217fbae81 100644 (file)
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -795,8 +795,14 @@ static noinline int lock_stripe_add(struct btrfs_raid_bio *rbio)
                    list_empty(&cur->plug_list) &&
                    test_bit(RBIO_CACHE_BIT, &cur->flags) &&
                    !test_bit(RBIO_RMW_LOCKED_BIT, &cur->flags)) {
+                       /*
+                        * Keep the hash-list reference: after the bucket lock
+                        * is released nothing but the cache holds cur, and a
+                        * concurrent cache removal would free it before the
+                        * remove_rbio_from_cache() below runs.  The reference
+                        * is dropped there, after the cache is done with it.
+                        */
                        list_del_init(&cur->hash_list);
-                       refcount_dec(&cur->refs);
 
                        steal_rbio(cur, rbio);
                        cache_drop = cur;
@@ -844,8 +850,10 @@ lockit:
        list_add(&rbio->hash_list, &h->hash_list);
 out:
        spin_unlock(&h->lock);
-       if (cache_drop)
+       if (cache_drop) {
                remove_rbio_from_cache(cache_drop);
+               free_raid_bio(cache_drop);
+       }
        if (freeit)
                free_raid_bio(freeit);
        return ret;