summaryrefslogtreecommitdiffstats
path: root/fs/kernfs/dir.c
diff options
context:
space:
mode:
authorTejun Heo <tj@kernel.org>2019-11-05 00:54:29 +0100
committerTejun Heo <tj@kernel.org>2019-11-12 17:18:03 +0100
commitb680b08171ebf890a4ebb7f82ada9959f4534ade (patch)
treee4dbcd63eb466e0efb09e76239057a2e2a03faab /fs/kernfs/dir.c
parentnetprio: use css ID instead of cgroup ID (diff)
downloadlinux-b680b08171ebf890a4ebb7f82ada9959f4534ade.tar.xz
linux-b680b08171ebf890a4ebb7f82ada9959f4534ade.zip
kernfs: use dumber locking for kernfs_find_and_get_node_by_ino()
kernfs_find_and_get_node_by_ino() uses RCU protection. It's currently a bit buggy because it can look up a node which hasn't been activated yet and thus may end up exposing a node that the kernfs user is still prepping. While it can be fixed by pushing it further in the current direction, it's already complicated and isn't clear whether the complexity is justified. The main use of kernfs_find_and_get_node_by_ino() is for exportfs operations. They aren't super hot and all the follow-up operations (e.g. mapping to path) use normal locking anyway. Let's switch to a dumber locking scheme and protect the lookup with kernfs_idr_lock. Signed-off-by: Tejun Heo <tj@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Namhyung Kim <namhyung@kernel.org>
Diffstat (limited to 'fs/kernfs/dir.c')
-rw-r--r--fs/kernfs/dir.c45
1 files changed, 9 insertions, 36 deletions
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 7d4af6cea2a6..798f0f03b62b 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -508,10 +508,6 @@ void kernfs_put(struct kernfs_node *kn)
struct kernfs_node *parent;
struct kernfs_root *root;
- /*
- * kernfs_node is freed with ->count 0, kernfs_find_and_get_node_by_ino
- * depends on this to filter reused stale node
- */
if (!kn || !atomic_dec_and_test(&kn->count))
return;
root = kernfs_root(kn);
@@ -646,11 +642,7 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
kn->id.ino = ret;
kn->id.generation = gen;
- /*
- * set ino first. This RELEASE is paired with atomic_inc_not_zero in
- * kernfs_find_and_get_node_by_ino
- */
- atomic_set_release(&kn->count, 1);
+ atomic_set(&kn->count, 1);
atomic_set(&kn->active, KN_DEACTIVATED_BIAS);
RB_CLEAR_NODE(&kn->rb);
@@ -716,38 +708,19 @@ struct kernfs_node *kernfs_find_and_get_node_by_ino(struct kernfs_root *root,
{
struct kernfs_node *kn;
- rcu_read_lock();
+ spin_lock(&kernfs_idr_lock);
+
kn = idr_find(&root->ino_idr, ino);
if (!kn)
- goto out;
+ goto err_unlock;
- /*
- * Since kernfs_node is freed in RCU, it's possible an old node for ino
- * is freed, but reused before RCU grace period. But a freed node (see
- * kernfs_put) or an incompletedly initialized node (see
- * __kernfs_new_node) should have 'count' 0. We can use this fact to
- * filter out such node.
- */
- if (!atomic_inc_not_zero(&kn->count)) {
- kn = NULL;
- goto out;
- }
-
- /*
- * The node could be a new node or a reused node. If it's a new node,
- * we are ok. If it's reused because of RCU (because of
- * SLAB_TYPESAFE_BY_RCU), the __kernfs_new_node always sets its 'ino'
- * before 'count'. So if 'count' is uptodate, 'ino' should be uptodate,
- * hence we can use 'ino' to filter stale node.
- */
- if (kn->id.ino != ino)
- goto out;
- rcu_read_unlock();
+ if (unlikely(!atomic_inc_not_zero(&kn->count)))
+ goto err_unlock;
+ spin_unlock(&kernfs_idr_lock);
return kn;
-out:
- rcu_read_unlock();
- kernfs_put(kn);
+err_unlock:
+ spin_unlock(&kernfs_idr_lock);
return NULL;
}