kernfs_perms_start() iterates the perms tree passed as @root (for sysfs
that is sysfs_root_kn), walking it with kernfs_next_recursive() ->
kernfs_parent(), and kernfs_perms_show() walks the parent chain of each
node likewise. kernfs_parent() is rcu_dereference_check(kn->__parent,
kernfs_root_is_locked(kn) || ...) and expects that node's kernfs rwsem
(or rename_lock, or RCU) to be held.
But the code took down_read() on kernfs_root(of->kn)->kernfs_rwsem, i.e.
the rwsem of the *control file*, not of the tree being walked. The perms
tree can live in a different kernfs instance than the control file - the
sysfs perms are shown via a cgroupfs file - so this is the wrong lock. On
a lockdep kernel it trips:
WARNING: suspicious RCU usage
fs/kernfs/kernfs-internal.h:131 suspicious rcu_dereference_check() usage!
kernfs_perms_show <- seq_read_iter <- vfs_read
and it is a real (if narrow) race: a concurrent sysfs add/remove/reparent
under the sysfs rwsem is not excluded by holding the cgroupfs rwsem.
Lock the rwsem of the iterated tree (kernfs_root(@root)) instead. This is
the lock kernfs_parent() expects and it excludes concurrent mutation of
the tree being walked. kernfs_perms_stop() takes @root too so it releases
the same rwsem.
Fixes: d0cb91d2dec2 ("ve/kernfs: fix kernfs locking in the control per-VE nodes
visibility code")
Feature: sysfs: per-CT entries visibility and permissions configuration
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <[email protected]>
---
fs/kernfs/ve.c | 21 ++++++++++++---------
fs/sysfs/ve.c | 2 +-
include/linux/kernfs-ve.h | 2 +-
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/fs/kernfs/ve.c b/fs/kernfs/ve.c
index e83ee5bec3bf..a89abc1f4040 100644
--- a/fs/kernfs/ve.c
+++ b/fs/kernfs/ve.c
@@ -163,11 +163,18 @@ void *kernfs_perms_start(struct seq_file *m, loff_t *ppos,
struct kernfs_node *root, struct kmapset_key *key)
{
struct ve_struct *ve = css_to_ve(seq_css(m));
- struct kernfs_open_file *of = m->private;
- struct kernfs_node *kn = of->kn;
- struct kernfs_root *root_for_sem = kernfs_root(kn);
+ struct kernfs_root *root_for_sem = kernfs_root(root);
+ struct kernfs_node *kn;
loff_t pos = *ppos;
+ /*
+ * We iterate the perms tree (@root), so hold that tree's
+ * kernfs_rwsem: it guards kernfs_parent()/kernfs_next_recursive()
+ * against a concurrent reparent and is the lock kernfs_parent()
+ * expects to be held. @root may live in a different kernfs instance
+ * than the control file (of->kn) - e.g. sysfs perms are shown via a
+ * cgroupfs file - so locking of->kn's root would be the wrong lock.
+ */
down_read(&root_for_sem->kernfs_rwsem);
for (kn = root; kn; kn = kernfs_next_recursive(kn)) {
if (kernfs_perms_shown(ve, kn, key) && !pos--)
@@ -190,13 +197,9 @@ void *kernfs_perms_next(struct seq_file *m, void *v,
loff_t *ppos,
return kn;
}
-void kernfs_perms_stop(struct seq_file *m, void *v)
+void kernfs_perms_stop(struct seq_file *m, void *v, struct kernfs_node *root)
{
- struct kernfs_open_file *of = m->private;
- struct kernfs_node *kn = of->kn;
- struct kernfs_root *root = kernfs_root(kn);
-
- up_read(&root->kernfs_rwsem);
+ up_read(&kernfs_root(root)->kernfs_rwsem);
}
int kernfs_perms_show(struct seq_file *m, void *v, struct kmapset_key *key)
diff --git a/fs/sysfs/ve.c b/fs/sysfs/ve.c
index eb941a1d5dd5..144a901547c0 100644
--- a/fs/sysfs/ve.c
+++ b/fs/sysfs/ve.c
@@ -55,7 +55,7 @@ static void *sysfs_perms_next(struct seq_file *m, void *v,
loff_t *ppos)
static void sysfs_perms_stop(struct seq_file *m, void *v)
{
- kernfs_perms_stop(m, v);
+ kernfs_perms_stop(m, v, sysfs_root_kn);
mutex_unlock(&sysfs_perms_mutex);
}
diff --git a/include/linux/kernfs-ve.h b/include/linux/kernfs-ve.h
index 2cb905918393..25837ff79e5b 100644
--- a/include/linux/kernfs-ve.h
+++ b/include/linux/kernfs-ve.h
@@ -24,7 +24,7 @@ void *kernfs_perms_start(struct seq_file *m, loff_t *ppos,
struct kernfs_node *root, struct kmapset_key *key);
void *kernfs_perms_next(struct seq_file *m, void *v, loff_t *ppos,
struct kmapset_key *key);
-void kernfs_perms_stop(struct seq_file *m, void *v);
+void kernfs_perms_stop(struct seq_file *m, void *v, struct kernfs_node *root);
int kernfs_perms_show(struct seq_file *m, void *v, struct kmapset_key *key);
--
2.47.1
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
