Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1379371 > unrolled thread

[PATCH] namespace: update event counter when umounting a deleted dentry

Back to article view | Back to linux.kernel

Contents

  [PATCH] namespace: update event counter when umounting a deleted dentry Andrey Ulanov <andreyu@google.com> - 2016-04-15 01:50 +0200
    Re: [PATCH] namespace: update event counter when umounting a deleted  dentry Al Viro <viro@ZenIV.linux.org.uk> - 2016-04-15 03:00 +0200

#1379371 — [PATCH] namespace: update event counter when umounting a deleted dentry

This change fixes a problem originally reported at:
https://github.com/coreos/bugs/issues/1153
You can find code that reproduces it at:
https://github.com/coreos/bugs/issues/1153#issuecomment-210201681

- m_start() in fs/namespace.c expects that ns->event is incremented each
  time a mount added or removed from ns->list.
- umount_tree() removes items from the list but does not increment event
  counter, expecting that it's done before the function is called.
- There are some codepaths that call umount_tree() without updating
  "event" counter. e.g. from __detach_mounts().
- When this happens m_start may reuse a cached mount structure that no
  longer belongs to ns->list (i.e. use after free which usually leads
  to infinite loop).

This change fixes the above problem by incrementing global event counter
before invoking umount_tree().

Signed-off-by: Andrey Ulanov <andreyu@google.com>
---
 fs/namespace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4fb1691..b06bb9a 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1562,6 +1562,7 @@ void __detach_mounts(struct dentry *dentry)
 		goto out_unlock;
 
 	lock_mount_hash();
+	event++;
 	while (!hlist_empty(&mp->m_list)) {
 		mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list);
 		if (mnt->mnt.mnt_flags & MNT_UMOUNT) {
@@ -1576,7 +1577,7 @@ out_unlock:
 	namespace_unlock();
 }
 
-/* 
+/*
  * Is the caller allowed to modify his namespace?
  */
 static inline bool may_mount(void)
@@ -1765,6 +1766,7 @@ void drop_collected_mounts(struct vfsmount *mnt)
 {
 	namespace_lock();
 	lock_mount_hash();
+	event++;
 	umount_tree(real_mount(mnt), UMOUNT_SYNC);
 	unlock_mount_hash();
 	namespace_unlock();
-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1379394 — Re: [PATCH] namespace: update event counter when umounting a deleted dentry

On Thu, Apr 14, 2016 at 04:48:32PM -0700, Andrey Ulanov wrote:
> +++ b/fs/namespace.c
> @@ -1562,6 +1562,7 @@ void __detach_mounts(struct dentry *dentry)
>  		goto out_unlock;
>  
>  	lock_mount_hash();
> +	event++;

That part makes sense.

> -/* 
> +/*
>   * Is the caller allowed to modify his namespace?
>   */
>  static inline bool may_mount(void)
> @@ -1765,6 +1766,7 @@ void drop_collected_mounts(struct vfsmount *mnt)
>  {
>  	namespace_lock();
>  	lock_mount_hash();
> +	event++;

... but what the hell is this one for?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web