From: Kent Overstreet Date: Tue, 21 Mar 2023 16:18:10 +0000 (-0400) Subject: bcachefs: Call bch2_path_put_nokeep() before bch2_path_put() X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=11f117374a2a353c378f8eccff8904d209643695;p=linux.git bcachefs: Call bch2_path_put_nokeep() before bch2_path_put() bch2_path_put_nokeep() is sketchy, and we should consider removing it: it unconditionally frees btree_paths once their ref hits 0. The assumption is that we only use it for paths that have never been visible outside the btree core btree code; i.e. higher level code will never be making assumptions about locking based on these paths. However, there's subtle brokenness with this approach: - If we call bch2_path_put(), then bch2_path_put_nokeep(), bch2_path_put() may free the first path on the assumption that we we have another path keeping a node locked - but then bch2_path_put_nokeep() just unconditionally frees it. The same bug may arise if we're calling bch2_path_put() and bch2_path_put_nokeep() on the same (refcounted) path, or two adjacent paths that point to the same btree node. This patch hacks around one of these bugs by calling bch2_path_put_nokeep() first in bch2_trans_iter_exit. Signed-off-by: Kent Overstreet --- diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c index 7b3e7f9368d1d..0a62f55a3aa8a 100644 --- a/fs/bcachefs/btree_iter.c +++ b/fs/bcachefs/btree_iter.c @@ -2722,12 +2722,12 @@ static inline void btree_path_list_add(struct btree_trans *trans, void bch2_trans_iter_exit(struct btree_trans *trans, struct btree_iter *iter) { - if (iter->path) - bch2_path_put(trans, iter->path, - iter->flags & BTREE_ITER_INTENT); if (iter->update_path) bch2_path_put_nokeep(trans, iter->update_path, iter->flags & BTREE_ITER_INTENT); + if (iter->path) + bch2_path_put(trans, iter->path, + iter->flags & BTREE_ITER_INTENT); if (iter->key_cache_path) bch2_path_put(trans, iter->key_cache_path, iter->flags & BTREE_ITER_INTENT);