Skip to content

Commit 80f130b

Browse files
kmjohansenMikulas Patocka
authored andcommitted
dm thin: make get_first_thin use rcu-safe list first function
The documentation in rculist.h explains the absence of list_empty_rcu() and cautions programmers against relying on a list_empty() -> list_first() sequence in RCU safe code. This is because each of these functions performs its own READ_ONCE() of the list head. This can lead to a situation where the list_empty() sees a valid list entry, but the subsequent list_first() sees a different view of list head state after a modification. In the case of dm-thin, this author had a production box crash from a GP fault in the process_deferred_bios path. This function saw a valid list head in get_first_thin() but when it subsequently dereferenced that and turned it into a thin_c, it got the inside of the struct pool, since the list was now empty and referring to itself. The kernel on which this occurred printed both a warning about a refcount_t being saturated, and a UBSAN error for an out-of-bounds cpuid access in the queued spinlock, prior to the fault itself. When the resulting kdump was examined, it was possible to see another thread patiently waiting in thin_dtr's synchronize_rcu. The thin_dtr call managed to pull the thin_c out of the active thins list (and have it be the last entry in the active_thins list) at just the wrong moment which lead to this crash. Fortunately, the fix here is straight forward. Switch get_first_thin() function to use list_first_or_null_rcu() which performs just a single READ_ONCE() and returns NULL if the list is already empty. This was run against the devicemapper test suite's thin-provisioning suites for delete and suspend and no regressions were observed. Signed-off-by: Krister Johansen <[email protected]> Fixes: b10ebd3 ("dm thin: fix rcu_read_lock being held in code that can sleep") Cc: [email protected] Acked-by: Ming-Hung Tsai <[email protected]> Signed-off-by: Mikulas Patocka <[email protected]>
1 parent 47f33c2 commit 80f130b

File tree

1 file changed

+2
-3
lines changed

1 file changed

+2
-3
lines changed

drivers/md/dm-thin.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,10 +2332,9 @@ static struct thin_c *get_first_thin(struct pool *pool)
23322332
struct thin_c *tc = NULL;
23332333

23342334
rcu_read_lock();
2335-
if (!list_empty(&pool->active_thins)) {
2336-
tc = list_entry_rcu(pool->active_thins.next, struct thin_c, list);
2335+
tc = list_first_or_null_rcu(&pool->active_thins, struct thin_c, list);
2336+
if (tc)
23372337
thin_get(tc);
2338-
}
23392338
rcu_read_unlock();
23402339

23412340
return tc;

0 commit comments

Comments
 (0)