afs: fix sget() races, close leak on umount
Al Viro [Sun, 12 Jun 2011 20:01:21 +0000 (16:01 -0400)]
* set ->s_fs_info in set() callback passed to sget()
* allocate the thing and set it up enough for afs_test_super() before
making it visible
* have it freed in ->kill_sb() (current tree simply leaks it)
* have ->put_super() leave ->s_fs_info->volume alone; it's too early for
dropping it; do that from ->kill_sb() after having called kill_anon_super().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

fs/afs/super.c

index fb240e8..b7d48d7 100644 (file)
@@ -31,8 +31,8 @@
 static void afs_i_init_once(void *foo);
 static struct dentry *afs_mount(struct file_system_type *fs_type,
                      int flags, const char *dev_name, void *data);
+static void afs_kill_super(struct super_block *sb);
 static struct inode *afs_alloc_inode(struct super_block *sb);
-static void afs_put_super(struct super_block *sb);
 static void afs_destroy_inode(struct inode *inode);
 static int afs_statfs(struct dentry *dentry, struct kstatfs *buf);
 
@@ -40,7 +40,7 @@ struct file_system_type afs_fs_type = {
        .owner          = THIS_MODULE,
        .name           = "afs",
        .mount          = afs_mount,
-       .kill_sb        = kill_anon_super,
+       .kill_sb        = afs_kill_super,
        .fs_flags       = 0,
 };
 
@@ -50,7 +50,6 @@ static const struct super_operations afs_super_ops = {
        .drop_inode     = afs_drop_inode,
        .destroy_inode  = afs_destroy_inode,
        .evict_inode    = afs_evict_inode,
-       .put_super      = afs_put_super,
        .show_options   = generic_show_options,
 };
 
@@ -282,19 +281,25 @@ static int afs_parse_device_name(struct afs_mount_params *params,
  */
 static int afs_test_super(struct super_block *sb, void *data)
 {
-       struct afs_mount_params *params = data;
+       struct afs_super_info *as1 = data;
        struct afs_super_info *as = sb->s_fs_info;
 
-       return as->volume == params->volume;
+       return as->volume == as1->volume;
+}
+
+static int afs_set_super(struct super_block *sb, void *data)
+{
+       sb->s_fs_info = data;
+       return set_anon_super(sb, NULL);
 }
 
 /*
  * fill in the superblock
  */
-static int afs_fill_super(struct super_block *sb, void *data)
+static int afs_fill_super(struct super_block *sb,
+                         struct afs_mount_params *params)
 {
-       struct afs_mount_params *params = data;
-       struct afs_super_info *as = NULL;
+       struct afs_super_info *as = sb->s_fs_info;
        struct afs_fid fid;
        struct dentry *root = NULL;
        struct inode *inode = NULL;
@@ -302,22 +307,11 @@ static int afs_fill_super(struct super_block *sb, void *data)
 
        _enter("");
 
-       /* allocate a superblock info record */
-       as = kzalloc(sizeof(struct afs_super_info), GFP_KERNEL);
-       if (!as) {
-               _leave(" = -ENOMEM");
-               return -ENOMEM;
-       }
-
-       afs_get_volume(params->volume);
-       as->volume = params->volume;
-
        /* fill in the superblock */
        sb->s_blocksize         = PAGE_CACHE_SIZE;
        sb->s_blocksize_bits    = PAGE_CACHE_SHIFT;
        sb->s_magic             = AFS_FS_MAGIC;
        sb->s_op                = &afs_super_ops;
-       sb->s_fs_info           = as;
        sb->s_bdi               = &as->volume->bdi;
 
        /* allocate the root inode and dentry */
@@ -326,7 +320,7 @@ static int afs_fill_super(struct super_block *sb, void *data)
        fid.unique      = 1;
        inode = afs_iget(sb, params->key, &fid, NULL, NULL);
        if (IS_ERR(inode))
-               goto error_inode;
+               return PTR_ERR(inode);
 
        if (params->autocell)
                set_bit(AFS_VNODE_AUTOCELL, &AFS_FS_I(inode)->flags);
@@ -342,16 +336,8 @@ static int afs_fill_super(struct super_block *sb, void *data)
        _leave(" = 0");
        return 0;
 
-error_inode:
-       ret = PTR_ERR(inode);
-       inode = NULL;
 error:
        iput(inode);
-       afs_put_volume(as->volume);
-       kfree(as);
-
-       sb->s_fs_info = NULL;
-
        _leave(" = %d", ret);
        return ret;
 }
@@ -367,6 +353,7 @@ static struct dentry *afs_mount(struct file_system_type *fs_type,
        struct afs_volume *vol;
        struct key *key;
        char *new_opts = kstrdup(options, GFP_KERNEL);
+       struct afs_super_info *as;
        int ret;
 
        _enter(",,%s,%p", dev_name, options);
@@ -399,12 +386,22 @@ static struct dentry *afs_mount(struct file_system_type *fs_type,
                ret = PTR_ERR(vol);
                goto error;
        }
-       params.volume = vol;
+
+       /* allocate a superblock info record */
+       as = kzalloc(sizeof(struct afs_super_info), GFP_KERNEL);
+       if (!as) {
+               ret = -ENOMEM;
+               afs_put_volume(vol);
+               goto error;
+       }
+       as->volume = vol;
 
        /* allocate a deviceless superblock */
-       sb = sget(fs_type, afs_test_super, set_anon_super, &params);
+       sb = sget(fs_type, afs_test_super, afs_set_super, as);
        if (IS_ERR(sb)) {
                ret = PTR_ERR(sb);
+               afs_put_volume(vol);
+               kfree(as);
                goto error;
        }
 
@@ -422,16 +419,16 @@ static struct dentry *afs_mount(struct file_system_type *fs_type,
        } else {
                _debug("reuse");
                ASSERTCMP(sb->s_flags, &, MS_ACTIVE);
+               afs_put_volume(vol);
+               kfree(as);
        }
 
-       afs_put_volume(params.volume);
        afs_put_cell(params.cell);
        kfree(new_opts);
        _leave(" = 0 [%p]", sb);
        return dget(sb->s_root);
 
 error:
-       afs_put_volume(params.volume);
        afs_put_cell(params.cell);
        key_put(params.key);
        kfree(new_opts);
@@ -439,18 +436,12 @@ error:
        return ERR_PTR(ret);
 }
 
-/*
- * finish the unmounting process on the superblock
- */
-static void afs_put_super(struct super_block *sb)
+static void afs_kill_super(struct super_block *sb)
 {
        struct afs_super_info *as = sb->s_fs_info;
-
-       _enter("");
-
+       kill_anon_super(sb);
        afs_put_volume(as->volume);
-
-       _leave("");
+       kfree(as);
 }
 
 /*