From 2a43a1393d40d37bd3db4281a7792e6e63b8e759 Mon Sep 17 00:00:00 2001 From: Zygo Blaxell Date: Fri, 18 Sep 2026 00:28:18 -0400 Subject: [PATCH] btrfs: stripe_alloc: hold a reference on the rbio being parked rbio_try_park() publishes the rbio on stripe_hash_table->parked and then keeps reading it: it drops parked_lock, queues the scan timer, and rechecks rbio_is_full() to reclaim a park that a concurrent merge has just filled. Nothing keeps the rbio alive across that. A parked rbio has exactly two references, the initial one and the one the stripe hash list took in lock_stripe_add(), and its completion consumes both. So the moment parked_lock is dropped, a flusher (btrfs_flush_parked_rbios(), unpark_ready_rbio(), or the park timer) can take the rbio, start its RMW, and have it complete through rbio_orig_end_io() -> unlock_stripe() -> free_raid_bio() -- while the parking thread is still inside rbio_try_park(), about to dereference it. lockdep sees it first, because rbio_is_full() takes rbio->bio_list_lock and a freed rbio's lockdep key is gone: INFO: trying to register non-static key. turning off the locking correctness validator. Workqueue: btrfs-rmw rmw_rbio_work register_lock_class+0x565/0x570 __lock_acquire+0x3a8/0x23d0 lock_acquire+0xe5/0x330 _raw_spin_lock+0x3b/0x90 rmw_rbio_work+0x256/0x400 and 62 ms later the same worker falls over the list_head next to it: list_del corruption, ffff8881810a56e0->next is NULL WARNING: CPU: 3 PID: 715967 at lib/list_debug.c:52 __list_del_entry_valid_or_report+0x7b/0x150 Workqueue: btrfs-rmw rmw_rbio_work That list_head is parked_node, at offset 0xe0 of struct btrfs_raid_bio. The task then took a NULL dereference holding parked_lock, and every other CPU piled into __pv_queued_spin_lock_slowpath() behind it: flushes from transaction commit and the ordered-extent waiters, the park timeout work, and other rmw_rbio_work()s. The box wedged with all CPUs spinning. Most interleavings survive by luck, which is why this is rare: the recheck tests RBIO_PARKED_BIT, and a legitimate unpark clears that bit under parked_lock, so the stale thread usually just sees it clear and returns. It stops being luck once the freed object has been reused. Take a reference before publishing and drop it after the recheck. It has to belong to the parking thread rather than to the parked list: a list-owned reference is inherited and dropped by whoever unparks the rbio, and that is precisely the thread racing us, so it protects everything except the window it was meant to protect. This one cannot be consumed by anyone else. Dropping it is never the last put -- a parked rbio still owns the stripe lock, and only the RMW a flusher starts after taking it off the list completes it -- so the free still happens where it always did. Assisted-by: Claude:claude-fable-5-1 --- fs/btrfs/raid56.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c index 60ad082942785..80d307202e574 100644 --- a/fs/btrfs/raid56.c +++ b/fs/btrfs/raid56.c @@ -894,6 +894,21 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio) rbio->park_deadline = jiffies + msecs_to_jiffies(timeout_ms); rbio->park_stuck_deadline = jiffies + msecs_to_jiffies(timeout_ms * 10); + /* + * Take a reference of our own before publishing. The moment + * parked_lock is dropped this rbio is visible to every flusher, and a + * flusher that claims it starts the RMW, which runs to completion and + * frees the rbio -- while we are still in this function, about to read + * it again in the fullness recheck below. Its other two references, + * the initial one and the stripe hash list's, are both consumed by + * that completion, so neither protects us. + * + * It has to be OUR reference, not one owned by the parked list: an + * unparker would inherit a list reference and drop it, and the + * unparker is exactly the thread racing us here. Nobody can consume + * this one but us. + */ + refcount_inc(&rbio->refs); list_add_tail(&rbio->parked_node, &table->parked); atomic_inc(&fs_info->stripe_parked_now); spin_unlock(&table->parked_lock); @@ -911,10 +926,18 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio) if (test_bit(RBIO_PARKED_BIT, &rbio->flags)) { rbio_unpark_locked(rbio); spin_unlock(&table->parked_lock); + free_raid_bio(rbio); return false; } spin_unlock(&table->parked_lock); } + /* + * Done reading it. Dropping our reference here cannot be the last + * one: a parked rbio still owns the stripe lock, and the only thing + * that completes it is the RMW a flusher starts after taking it off + * the list. + */ + free_raid_bio(rbio); return true; } -- 2.53.0