diff mbox

fs: fix a posible leak of allocated superblock

Message ID 1458043730-14296-1-git-send-email-ptikhomirov@virtuozzo.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Pavel Tikhomirov March 15, 2016, 12:08 p.m. UTC
We probably need to fix superblock leak in patch (v4 "fs: Add user
namesapace member to struct super_block"):

Imagine posible code path in sget_userns: we iterate through
type->fs_supers and do not find suitable sb, we drop sb_lock to
allocate s and go to retry. After we dropped sb_lock some other
task from different userns takes sb_lock, it is already in retry
stage and has s allocated, so it puts its s in type->fs_supers
list. So in retry we will find these sb in list and check it has
a different userns, and finally we will return without freeing s.

Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 fs/super.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Seth Forshee March 15, 2016, 1:32 p.m. UTC | #1
On Tue, Mar 15, 2016 at 03:08:50PM +0300, Pavel Tikhomirov wrote:
> We probably need to fix superblock leak in patch (v4 "fs: Add user
> namesapace member to struct super_block"):
> 
> Imagine posible code path in sget_userns: we iterate through
> type->fs_supers and do not find suitable sb, we drop sb_lock to
> allocate s and go to retry. After we dropped sb_lock some other
> task from different userns takes sb_lock, it is already in retry
> stage and has s allocated, so it puts its s in type->fs_supers
> list. So in retry we will find these sb in list and check it has
> a different userns, and finally we will return without freeing s.
> 
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

Good catch.

Acked-by: Seth Forshee <seth.forshee@canonical.com>
diff mbox

Patch

diff --git a/fs/super.c b/fs/super.c
index b4ee02b..24771b5 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -458,6 +458,10 @@  struct super_block *sget_userns(struct file_system_type *type,
 				continue;
 			if (user_ns != old->s_user_ns) {
 				spin_unlock(&sb_lock);
+				if (s) {
+					up_write(&s->s_umount);
+					destroy_super(s);
+				}
 				return ERR_PTR(-EBUSY);
 			}
 			if (!grab_super(old))