fs: umount on symlinlk leaks mnt count

PATCH Superseded Vasily Averin 4 participants 7 messages Jul 20, 2014 – Jul 24, 2014 1 message not in the archive VFS

lore

Share & tools

lore

Vasily Averin

author

2014-07-20 07:32 UTC #0 lore raw

PATCH v2 1/1 fs: umount on symlinlk leaks mnt count

v2: mntget(path->mnt) should not be called if mountpoint_last() returns 1.

Currently umount on symlink blocks following umount:

/vz is separate mount

# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir

# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)

# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)

In this case mountpoint_last() gets an extra refcount on path->mnt

Signed-off-by Vasily Averin <[email protected]>

fs/namei.c +2 -1

index 985c6f3..9eb787e 100644
@@ -2256,9 +2256,10 @@ done:
 		goto out;
 	}
 	path->dentry = dentry;
-	path->mnt = mntget(nd->path.mnt);
+	path->mnt = nd->path.mnt;
 	if (should_follow_link(dentry, nd->flags & LOOKUP_FOLLOW))
 		return 1;
+	mntget(path->mnt);
 	follow_mount(path);
 	error = 0;
 out:

Richard Weinberger

2014-07-20 10:48 UTC #1 lore raw

Quoted 13 lines · Vasily Averin

v2: mntget(path->mnt) should not be called if mountpoint_last() returns 1.
Currently umount on symlink blocks following umount:
/vz is separate mount
# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir
# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)

Quoted 7 lines

# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)
In this case mountpoint_last() gets an extra refcount on path->mnt
Signed-off-by Vasily Averin <[email protected]>

Sounds reasonable to me.
Can you please send the patch inline and not as attachment?

Signature · 2 lines

Thanks,
//richard

A message that is not in the archive (sent off-list or not yet received): [email protected]

Vasily Averin

author

2014-07-20 12:38 UTC #3 lore raw

PATCH v3 1/1 fs: umount on symlink leaks mnt count

v3: patch inline

Currently umount on symlink blocks following umount:

/vz is separate mount

drwxr-xr-x. 2 root root 4096 Jul 19 01:14 testdir
lrwxrwxrwx. 1 root root 11 Jul 19 01:16 testlink -> /vz/testdir

umount: /vz/testlink: not mounted (expected)

umount: /vz: device is busy. (unexpected)

In this case mountpoint_last() gets an extra refcount on path->mnt

Signed-off-by Vasily Averin <[email protected]>

---
 fs/namei.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

fs/namei.c +2 -1

index 985c6f3..9eb787e 100644
@@ -2256,9 +2256,10 @@ done:
 		goto out;
 	}
 	path->dentry = dentry;
-	path->mnt = mntget(nd->path.mnt);
+	path->mnt = nd->path.mnt;
 	if (should_follow_link(dentry, nd->flags & LOOKUP_FOLLOW))
 		return 1;
+	mntget(path->mnt);
 	follow_mount(path);
 	error = 0;
 out:

Signature · 1 line

1.7.5.4

Vasily Averin

author

2014-07-21 08:30 UTC #4 lore raw

PATCH v4 1/1 fs: umount on symlink leaks mnt count

In reply to a message not in the archive

v4: description corrected
v3: patch inline

Currently umount on symlink blocks following umount:

/vz is separate mount

# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir
# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)

# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)

In this case mountpoint_last() gets an extra refcount on path->mnt

Signed-off-by Vasily Averin <[email protected]>

---
 fs/namei.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

fs/namei.c +2 -1

index 985c6f3..9eb787e 100644
@@ -2256,9 +2256,10 @@ done:
 		goto out;
 	}
 	path->dentry = dentry;
-	path->mnt = mntget(nd->path.mnt);
+	path->mnt = nd->path.mnt;
 	if (should_follow_link(dentry, nd->flags & LOOKUP_FOLLOW))
 		return 1;
+	mntget(path->mnt);
 	follow_mount(path);
 	error = 0;
 out:

Signature · 1 line

1.7.5.4

Jeff Layton

2014-07-21 12:10 UTC #5 lore raw

Re: [PATCH v4] fs: umount on symlink leaks mnt count

Quoted 30 lines · On Mon, 21 Jul 2014 12:30:23 +0400 Vasily Averin <[email protected]> wrote:

v4: description corrected
v3: patch inline
Currently umount on symlink blocks following umount:
/vz is separate mount
# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir
# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)
# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)
In this case mountpoint_last() gets an extra refcount on path->mnt
Signed-off-by Vasily Averin <[email protected]>
---
 fs/namei.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
fs/namei.c +0 -0
index 985c6f3..9eb787e 100644
@@ -2256,9 +2256,10 @@ done:
 		goto out;

fs/namei.c done: · line 2265

Quoted 10 lines

fs/namei.c +2 -1

 	}
 	path->dentry = dentry;
-	path->mnt = mntget(nd->path.mnt);
+	path->mnt = nd->path.mnt;
 	if (should_follow_link(dentry, nd->flags & LOOKUP_FOLLOW))
 		return 1;
+	mntget(path->mnt);
 	follow_mount(path);
 	error = 0;
 out:

Looks correct, I think...

Acked-by:

Signature · 1 line

Jeff Layton <[email protected]>

Ian Kent

Acked-by on v4 1/1

2014-07-23 06:06 UTC #6 lore raw

Re: [PATCH v4] fs: umount on symlink leaks mnt count

In reply to Vasily Averin

Quoted 13 lines · Vasily Averin

v4: description corrected
v3: patch inline
Currently umount on symlink blocks following umount:
/vz is separate mount
# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir
# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)

Quoted 7 lines

# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)
In this case mountpoint_last() gets an extra refcount on path->mnt
Signed-off-by Vasily Averin <[email protected]>

Acked-by Ian Kent <[email protected]>

I was working on this too.
I prefer this patch over my own.
Haven't tested it yet but it is very similar to the patch I was working with which had seen some basic testing.

Quoted 20 lines

---
 fs/namei.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

fs/namei.c +2 -1

index 985c6f3..9eb787e 100644
@@ -2256,9 +2256,10 @@ done:
 		goto out;
 	}
 	path->dentry = dentry;
-	path->mnt = mntget(nd->path.mnt);
+	path->mnt = nd->path.mnt;
 	if (should_follow_link(dentry, nd->flags & LOOKUP_FOLLOW))
 		return 1;
+	mntget(path->mnt);
 	follow_mount(path);
 	error = 0;
 out:

Vasily Averin

author

2014-07-24 05:59 UTC #7 lore raw

Re: [PATCH v4] fs: umount on symlink leaks mnt count

CVE-2014-5045 was assigned for this issue

Quoted 20 lines · Ian Kent

Quoted 10 lines · Vasily Averin
Currently umount on symlink blocks following umount:
/vz is separate mount
# ls /vz/ -al | grep test
drwxr-xr-x.  2 root root       4096 Jul 19 01:14 testdir
lrwxrwxrwx.  1 root root         11 Jul 19 01:16 testlink -> /vz/testdir
# umount -l /vz/testlink
umount: /vz/testlink: not mounted (expected)
Quoted 7 lines
# lsof /vz
# umount /vz
umount: /vz: device is busy. (unexpected)
In this case mountpoint_last() gets an extra refcount on path->mnt
Signed-off-by Vasily Averin <[email protected]>
Acked-by Ian Kent <[email protected]>