[PATCH] vfs: document locking for mnt_notify_add()

[Jann Horn]

The locking in mnt_notify_add(), which was introduced in commit
bf630c401641 ("vfs: add notifications for mount attach and detach"), is a
bit gnarly.
notify_list is protected by namespace_lock, but there are cases where
mnt_notify_add() is called without holding namespace_lock, for example:

    __do_sys_fsmount -> mnt_add_to_ns -> mnt_notify_add

Luckily, in cases where the namespace_lock isn't held, the namespace is
always freshly created and can't have any fsnotify marks yet, which means
the notify_list isn't actually accessed.

The existing comment claims that not accessing the notify_list in these
cases is merely an optimization, which is wrong. Fix the comment, and add a
locking assertion.

To allow mnt_notify_add() to reference the namespace_sem, move it into
fs/namespace.c.

Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>
---
I'm sending this patch because I spent some time staring at this
trying to figure out if this was buggy or not.

I don't know if this is working as intended or working by accident,
and it might be nice if this was cleaned up to have simpler locking;
but for now, document what's going on for the next person who stares
at this code.
---
 fs/mount.h     | 10 +---------
 fs/namespace.c | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index e0816c11a198..99016db2f408 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -219,15 +219,7 @@ static inline struct mnt_namespace *to_mnt_ns(struct ns_common *ns)
 }
 
 #ifdef CONFIG_FSNOTIFY
-static inline void mnt_notify_add(struct mount *m)
-{
-	/* Optimize the case where there are no watches */
-	if ((m->mnt_ns && m->mnt_ns->n_fsnotify_marks) ||
-	    (m->prev_ns && m->prev_ns->n_fsnotify_marks))
-		list_add_tail(&m->to_notify, &notify_list);
-	else
-		m->prev_ns = m->mnt_ns;
-}
+void mnt_notify_add(struct mount *m);
 #else
 static inline void mnt_notify_add(struct mount *m)
 {
diff --git a/fs/namespace.c b/fs/namespace.c
index fe919abd2f01..56f130b49e58 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -6431,6 +6431,25 @@ bool mnt_may_suid(struct vfsmount *mnt)
 	       current_in_userns(mnt->mnt_sb->s_user_ns);
 }
 
+#ifdef CONFIG_FSNOTIFY
+void mnt_notify_add(struct mount *m)
+{
+	/*
+	 * notify_list is protected by namespace_sem.
+	 * It is possible to call this function without holding namespace_sem,
+	 * but in those cases, the mount is associated with a new mount
+	 * namespace that can't have any fanotify marks yet.
+	 */
+	if ((m->mnt_ns && m->mnt_ns->n_fsnotify_marks) ||
+	    (m->prev_ns && m->prev_ns->n_fsnotify_marks)) {
+		rwsem_assert_held_write(&namespace_sem);
+		list_add_tail(&m->to_notify, &notify_list);
+	} else {
+		m->prev_ns = m->mnt_ns;
+	}
+}
+#endif
+
 static struct ns_common *mntns_get(struct task_struct *task)
 {
 	struct ns_common *ns = NULL;

---
base-commit: 8fde5d1d47f69db6082dfa34500c27f8485389a5
change-id: 20260529-vfs-mnt-notify-comment-4369e0558f0b

Best regards,
--  
Jann Horn <jannh@xxxxxxxxxx>