[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240919073603.1827-1-bottaawesome633@gmail.com>
Date: Thu, 19 Sep 2024 10:36:03 +0300
From: Ahmed Ehab <bottaawesome633@...il.com>
To: linux-kernel@...r.kernel.org
Cc: Kent Overstreet <kent.overstreet@...ux.dev>,
linux-bcachefs@...r.kernel.org,
syzkaller@...glegroups.com,
syzbot+f7c41a878676b72c16a6@...kaller.appspotmail.com
Subject: [PATCH] bcachefs: Relocate rcu_read_lock()/rcu_read_unlock() to snapshot_t()
Syzbot reports a problem that a warning is triggered due to suspicious
use of rcu_dereference_check(). That is triggered by a call of
bch2_snapshot_tree_oldest_subvol().
The cause of the warning is that the rcu_read_lock() is called in wrapper
methods instead of calling it directly before calling rcu_dereference()
in snapshot_t().For example in this case, snapshot_t() is called
directly from bch2_snapshot_tree_oldest_subvol() without holding the
read lock. This also results in duplicating the rcu_read_lock()
and rcu_read_unlock() calls, which may lead to future errors in the case
of forgetting to hold the read locks as in this case.
To fix this, move rcu_read_lock() and rcu_read_unlock() to snapshot_t().
This will make sure that rcu_dereference_check() is never called without
holding the read lock.
Reported-by: <syzbot+f7c41a878676b72c16a6@...kaller.appspotmail.com>
Signed-off-by: Ahmed Ehab <bottaawesome633@...il.com>
---
fs/bcachefs/snapshot.h | 26 +++++++-------------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/fs/bcachefs/snapshot.h b/fs/bcachefs/snapshot.h
index eb5ef64221d6..04f18fac56fe 100644
--- a/fs/bcachefs/snapshot.h
+++ b/fs/bcachefs/snapshot.h
@@ -42,15 +42,19 @@ static inline struct snapshot_t *__snapshot_t(struct snapshot_table *t, u32 id)
static inline const struct snapshot_t *snapshot_t(struct bch_fs *c, u32 id)
{
- return __snapshot_t(rcu_dereference(c->snapshots), id);
+ struct snapshot_table *temp;
+
+ rcu_read_lock();
+ temp = rcu_dereference(c->snapshots);
+ rcu_read_unlock();
+
+ return __snapshot_t(temp, id);
}
static inline u32 bch2_snapshot_tree(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
const struct snapshot_t *s = snapshot_t(c, id);
id = s ? s->tree : 0;
- rcu_read_unlock();
return id;
}
@@ -63,9 +67,7 @@ static inline u32 __bch2_snapshot_parent_early(struct bch_fs *c, u32 id)
static inline u32 bch2_snapshot_parent_early(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
id = __bch2_snapshot_parent_early(c, id);
- rcu_read_unlock();
return id;
}
@@ -89,19 +91,15 @@ static inline u32 __bch2_snapshot_parent(struct bch_fs *c, u32 id)
static inline u32 bch2_snapshot_parent(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
id = __bch2_snapshot_parent(c, id);
- rcu_read_unlock();
return id;
}
static inline u32 bch2_snapshot_nth_parent(struct bch_fs *c, u32 id, u32 n)
{
- rcu_read_lock();
while (n--)
id = __bch2_snapshot_parent(c, id);
- rcu_read_unlock();
return id;
}
@@ -112,10 +110,8 @@ static inline u32 bch2_snapshot_root(struct bch_fs *c, u32 id)
{
u32 parent;
- rcu_read_lock();
while ((parent = __bch2_snapshot_parent(c, id)))
id = parent;
- rcu_read_unlock();
return id;
}
@@ -128,19 +124,15 @@ static inline u32 __bch2_snapshot_equiv(struct bch_fs *c, u32 id)
static inline u32 bch2_snapshot_equiv(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
id = __bch2_snapshot_equiv(c, id);
- rcu_read_unlock();
return id;
}
static inline int bch2_snapshot_is_internal_node(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
const struct snapshot_t *s = snapshot_t(c, id);
int ret = s ? s->children[0] : -BCH_ERR_invalid_snapshot_node;
- rcu_read_unlock();
return ret;
}
@@ -157,9 +149,7 @@ static inline u32 bch2_snapshot_depth(struct bch_fs *c, u32 parent)
{
u32 depth;
- rcu_read_lock();
depth = parent ? snapshot_t(c, parent)->depth + 1 : 0;
- rcu_read_unlock();
return depth;
}
@@ -175,10 +165,8 @@ static inline bool bch2_snapshot_is_ancestor(struct bch_fs *c, u32 id, u32 ances
static inline bool bch2_snapshot_has_children(struct bch_fs *c, u32 id)
{
- rcu_read_lock();
const struct snapshot_t *t = snapshot_t(c, id);
bool ret = t && (t->children[0]|t->children[1]) != 0;
- rcu_read_unlock();
return ret;
}
--
2.46.0
Powered by blists - more mailing lists