Skip to content

Commit

Permalink
afs: fix sget() races, close leak on umount
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Al Viro committed Jun 12, 2011
1 parent d251ed2 commit dde194a
Showing 1 changed file with 32 additions and 41 deletions.
73 changes: 32 additions & 41 deletions fs/afs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@
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);

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,
};

Expand All @@ -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,
};

Expand Down Expand Up @@ -282,42 +281,37 @@ 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;
int ret;

_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 */
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -422,35 +419,29 @@ 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);
_leave(" = %d", ret);
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);
}

/*
Expand Down

0 comments on commit dde194a

Please sign in to comment.