kernfs_rename_ns() only takes kernfs_rename_lock when the rename moves
the node to a new parent. A rename that keeps the same parent, like
renaming a network interface, changes kernfs_node::name with only
kernfs_rwsem held. So the lock protects ->__parent but not ->name, and
a reader that wants a stable name has to take kernfs_rwsem, the same
lock every path lookup needs.
That also makes for a small but real bug. kernfs_path_from_node()
takes kernfs_rename_lock for reading, and kernfs_path_from_node_locked()
then reads the name of each ancestor. It reads each one once, so a
single same-parent rename only moves the answer from the old path to the
new one, but two of them landing inside one walk build a path that never
existed:
CPU0 CPU1
kernfs_path_from_node() on /a/b/c
reads the name of a, gets "a"
renames a to a2
renames b to b2
reads the name of b, gets "b2"
returns "/a/b2/c"
This hits roots without KERNFS_ROOT_INVARIANT_PARENT: sysfs, where the
bad path can reach sysfs_warn_dup() and pr_cont_kernfs_path(), and
resctrl, which renames a mon group inside its mon_groups directory.
cgroup sets the flag, so it skips the lock and reads names under RCU
alone; that case needs something else and is not addressed here.
So take the lock in both cases, and let kernfs_rcu_name() accept it the
way kernfs_parent() already does for ->__parent. Same-parent renames
are rare, the lock is per filesystem, and the locked section is at most
three stores. It also gives a future rename sequence counter one place
to sit that covers every rename.
Fixes: 741c10b096bc ("kernfs: Use RCU to access kernfs_node::name.")
Signed-off-by: Shakeel Butt <[email protected]>
---
fs/kernfs/dir.c | 28 +++++++++++++++-------------
fs/kernfs/kernfs-internal.h | 9 ++++++++-
2 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index cd7a8ff8b6b2..214c97130a8a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1808,6 +1808,7 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct
kernfs_node *new_parent,
struct kernfs_node *old_parent;
struct kernfs_root *root;
const char *old_name;
+ bool reparent;
int error;
/* can't move or rename root */
@@ -1857,25 +1858,26 @@ int kernfs_rename_ns(struct kernfs_node *kn, struct
kernfs_node *new_parent,
*/
kernfs_unlink_sibling(kn);
- /* rename_lock protects ->parent accessors */
- if (old_parent != new_parent) {
+ reparent = old_parent != new_parent;
+ if (reparent)
kernfs_get(new_parent);
- write_lock_irq(&root->kernfs_rename_lock);
+ /*
+ * kernfs_rename_lock protects ->__parent, ->ns and ->name, so take it
+ * even when the parent does not change.
+ */
+ write_lock_irq(&root->kernfs_rename_lock);
+
+ if (reparent)
rcu_assign_pointer(kn->__parent, new_parent);
+ WRITE_ONCE(kn->ns, new_ns);
+ if (new_name)
+ rcu_assign_pointer(kn->name, new_name);
- WRITE_ONCE(kn->ns, new_ns);
- if (new_name)
- rcu_assign_pointer(kn->name, new_name);
+ write_unlock_irq(&root->kernfs_rename_lock);
- write_unlock_irq(&root->kernfs_rename_lock);
+ if (reparent)
kernfs_put(old_parent);
- } else {
- /* name assignment is RCU protected, parent is the same */
- WRITE_ONCE(kn->ns, new_ns);
- if (new_name)
- rcu_assign_pointer(kn->name, new_name);
- }
kn->hash = kernfs_name_hash(new_name ?: old_name, kn->ns);
kernfs_link_sibling(kn);
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 20a0cf42ba8d..1609c1519698 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -117,7 +117,14 @@ static inline bool kernfs_rename_is_locked(const struct
kernfs_node *kn)
static inline const char *kernfs_rcu_name(const struct kernfs_node *kn)
{
- return rcu_dereference_check(kn->name, kernfs_root_is_locked(kn));
+ /*
+ * Like kernfs_node::__parent below, the name is only replaced under
+ * both kernfs_root::kernfs_rwsem and kernfs_root::kernfs_rename_lock,
+ * so either one keeps it, and the string it points at, stable.
+ */
+ return rcu_dereference_check(kn->name,
+ kernfs_root_is_locked(kn) ||
+ kernfs_rename_is_locked(kn));
}
static inline struct kernfs_node *kernfs_parent(const struct kernfs_node *kn)
--
2.53.0-Meta
