Commit 1ce88cf4 authored by Miklos Szeredi's avatar Miklos Szeredi Committed by Linus Torvalds

[PATCH] namespace.c: fix race in mark_mounts_for_expiry()

This patch fixes a race found by Ram in mark_mounts_for_expiry() in
fs/namespace.c.

The bug can only be triggered with simultaneous exiting of a process having
a private namespace, and expiry of a mount from within that namespace.
It's practically impossible to trigger, and I haven't even tried.  But
still, a bug is a bug.

The race happens when put_namespace() is called by another task, while
mark_mounts_for_expiry() is between atomic_read() and get_namespace().  In
that case get_namespace() will be called on an already dead namespace with
unforeseeable results.

The solution was suggested by Al Viro, with his own words:

      Instead of screwing with atomic_read() in there, why don't we
      simply do the following:
      	a) atomic_dec_and_lock() in put_namespace()
      	b) __put_namespace() called without dropping lock
      	c) the first thing done by __put_namespace would be
      struct vfsmount *root = namespace->root;
      namespace->root = NULL;
      spin_unlock(...);
      ....
      umount_tree(root);
      ...
      	d) check in mark_... would be simply namespace && namespace->root.

      And we are all set; no screwing around with atomic_read(), no magic
      at all.  Dying namespace gets NULL ->root.
      All changes of ->root happen under spinlock.
      If under a spinlock we see non-NULL ->mnt_namespace, it won't be
      freed until we drop the lock (we will set ->mnt_namespace to NULL
      under that lock before we get to freeing namespace).
      If under a spinlock we see non-NULL ->mnt_namespace and
      ->mnt_namespace->root, we can grab a reference to namespace and be
      sure that it won't go away.
Signed-off-by: default avatarMiklos Szeredi <miklos@szeredi.hu>
Acked-by: default avatarAl Viro <viro@parcelfarce.linux.theplanet.co.uk>
Acked-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 202322e6
...@@ -869,7 +869,7 @@ void mark_mounts_for_expiry(struct list_head *mounts) ...@@ -869,7 +869,7 @@ void mark_mounts_for_expiry(struct list_head *mounts)
/* don't do anything if the namespace is dead - all the /* don't do anything if the namespace is dead - all the
* vfsmounts from it are going away anyway */ * vfsmounts from it are going away anyway */
namespace = mnt->mnt_namespace; namespace = mnt->mnt_namespace;
if (!namespace || atomic_read(&namespace->count) <= 0) if (!namespace || !namespace->root)
continue; continue;
get_namespace(namespace); get_namespace(namespace);
...@@ -1450,9 +1450,12 @@ void __init mnt_init(unsigned long mempages) ...@@ -1450,9 +1450,12 @@ void __init mnt_init(unsigned long mempages)
void __put_namespace(struct namespace *namespace) void __put_namespace(struct namespace *namespace)
{ {
struct vfsmount *root = namespace->root;
namespace->root = NULL;
spin_unlock(&vfsmount_lock);
down_write(&namespace->sem); down_write(&namespace->sem);
spin_lock(&vfsmount_lock); spin_lock(&vfsmount_lock);
umount_tree(namespace->root); umount_tree(root);
spin_unlock(&vfsmount_lock); spin_unlock(&vfsmount_lock);
up_write(&namespace->sem); up_write(&namespace->sem);
kfree(namespace); kfree(namespace);
......
...@@ -17,7 +17,8 @@ extern void __put_namespace(struct namespace *namespace); ...@@ -17,7 +17,8 @@ extern void __put_namespace(struct namespace *namespace);
static inline void put_namespace(struct namespace *namespace) static inline void put_namespace(struct namespace *namespace)
{ {
if (atomic_dec_and_test(&namespace->count)) if (atomic_dec_and_lock(&namespace->count, &vfsmount_lock))
/* releases vfsmount_lock */
__put_namespace(namespace); __put_namespace(namespace);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment