path_overmount(): avoid false negatives

Holding namespace_sem is enough to make sure that result remains valid.
It is *not* enough to avoid false negatives from __lookup_mnt().  Mounts
can be unhashed outside of namespace_sem (stuck children getting detached
on final mntput() of lazy-umounted mount) and having an unrelated mount
removed from the hash chain while we traverse it may end up with false
negative from __lookup_mnt().  We need to sample and recheck the seqlock
component of mount_lock...

Bug predates the introduction of path_overmount() - it had come from
the code in finish_automount() that got abstracted into that helper.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Fixes: 26df6034fdb2 ("fix automount/automount race properly")
Fixes: 6ac392815628 ("fs: allow to mount beneath top mount")
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namespace.c b/fs/namespace.c
index a33553bc12d0681d9db07b0183735fc277d978b7..1722deadfb88051ade586cf964b801587d33ec1d 100644 (file)
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3478,18 +3478,25 @@ out:
  * Check if path is overmounted, i.e., if there's a mount on top of
  * @path->mnt with @path->dentry as mountpoint.
  *
- * Context: This function expects namespace_lock() to be held.
+ * Context: namespace_sem must be held at least shared.
+ * MUST NOT be called under lock_mount_hash() (there one should just
+ * call __lookup_mnt() and check if it returns NULL).
  * Return: If path is overmounted true is returned, false if not.
  */
 static inline bool path_overmounted(const struct path *path)
 {
+       unsigned seq = read_seqbegin(&mount_lock);
+       bool no_child;
+
        rcu_read_lock();
-       if (unlikely(__lookup_mnt(path->mnt, path->dentry))) {
-               rcu_read_unlock();
-               return true;
-       }
+       no_child = !__lookup_mnt(path->mnt, path->dentry);
        rcu_read_unlock();
-       return false;
+       if (need_seqretry(&mount_lock, seq)) {
+               read_seqlock_excl(&mount_lock);
+               no_child = !__lookup_mnt(path->mnt, path->dentry);
+               read_sequnlock_excl(&mount_lock);
+       }
+       return unlikely(!no_child);
 }
 
 /**