Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

closures: Fix race in closure_sync()

As pointed out by Linus, closure_sync() was racy; we could skip blocking
immediately after a get() and a put(), but then that would skip any
barrier corresponding to the other thread's put() barrier.

To fix this, always do the full __closure_sync() sequence whenever any
get() has happened and the closure might have been used by other
threads.

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

+13 -1
+1
fs/bcachefs/fs-io-direct.c
··· 113 113 } else { 114 114 atomic_set(&dio->cl.remaining, 115 115 CLOSURE_REMAINING_INITIALIZER + 1); 116 + dio->cl.closure_get_happened = true; 116 117 } 117 118 118 119 dio->req = req;
+9 -1
include/linux/closure.h
··· 154 154 struct closure *parent; 155 155 156 156 atomic_t remaining; 157 + bool closure_get_happened; 157 158 158 159 #ifdef CONFIG_DEBUG_CLOSURES 159 160 #define CLOSURE_MAGIC_DEAD 0xc054dead ··· 186 185 */ 187 186 static inline void closure_sync(struct closure *cl) 188 187 { 189 - if (closure_nr_remaining(cl) != 1) 188 + #ifdef CONFIG_DEBUG_CLOSURES 189 + BUG_ON(closure_nr_remaining(cl) != 1 && !cl->closure_get_happened); 190 + #endif 191 + 192 + if (cl->closure_get_happened) 190 193 __closure_sync(cl); 191 194 } 192 195 ··· 262 257 */ 263 258 static inline void closure_get(struct closure *cl) 264 259 { 260 + cl->closure_get_happened = true; 261 + 265 262 #ifdef CONFIG_DEBUG_CLOSURES 266 263 BUG_ON((atomic_inc_return(&cl->remaining) & 267 264 CLOSURE_REMAINING_MASK) <= 1); ··· 286 279 closure_get(parent); 287 280 288 281 atomic_set(&cl->remaining, CLOSURE_REMAINING_INITIALIZER); 282 + cl->closure_get_happened = false; 289 283 290 284 closure_debug_create(cl); 291 285 closure_set_ip(cl);
+3
lib/closure.c
··· 23 23 if (!r) { 24 24 smp_acquire__after_ctrl_dep(); 25 25 26 + cl->closure_get_happened = false; 27 + 26 28 if (cl->fn && !(flags & CLOSURE_DESTRUCTOR)) { 27 29 atomic_set(&cl->remaining, 28 30 CLOSURE_REMAINING_INITIALIZER); ··· 94 92 if (atomic_read(&cl->remaining) & CLOSURE_WAITING) 95 93 return false; 96 94 95 + cl->closure_get_happened = true; 97 96 closure_set_waiting(cl, _RET_IP_); 98 97 atomic_add(CLOSURE_WAITING + 1, &cl->remaining); 99 98 llist_add(&cl->list, &waitlist->list);