Skip to content

Commit

Permalink
hfsplus: add error checking for hfs_find_init()
Browse files Browse the repository at this point in the history 
hfs_find_init() may fail with ENOMEM, but there are places, where
the returned value is not checked. The consequences can be very
unpleasant, e.g. kfree uninitialized pointer and
inappropriate mutex unlocking.

The patch adds checks for errors in hfs_find_init().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Christoph Hellwig <hch@lst.de>
  • Loading branch information
Alexey Khoroshilov authored and Christoph Hellwig committed Jul 7, 2011
1 parent c6d5f5f commit 5bd9d99
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 26 deletions.
14 changes: 10 additions & 4 deletions fs/hfsplus/catalog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,9 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir,

dprint(DBG_CAT_MOD, "create_cat: %s,%u(%d)\n",
str->name, cnid, inode->i_nlink);
hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
if (err)
return err;

hfsplus_cat_build_key(sb, fd.search_key, cnid, NULL);
entry_size = hfsplus_fill_cat_thread(sb, &entry,
Expand Down Expand Up @@ -269,7 +271,9 @@ int hfsplus_delete_cat(u32 cnid, struct inode *dir, struct qstr *str)

dprint(DBG_CAT_MOD, "delete_cat: %s,%u\n",
str ? str->name : NULL, cnid);
hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
if (err)
return err;

if (!str) {
int len;
Expand Down Expand Up @@ -347,12 +351,14 @@ int hfsplus_rename_cat(u32 cnid,
struct hfs_find_data src_fd, dst_fd;
hfsplus_cat_entry entry;
int entry_size, type;
int err = 0;
int err;

dprint(DBG_CAT_MOD, "rename_cat: %u - %lu,%s - %lu,%s\n",
cnid, src_dir->i_ino, src_name->name,
dst_dir->i_ino, dst_name->name);
hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &src_fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &src_fd);
if (err)
return err;
dst_fd = src_fd;

/* find the old dir entry and read the data */
Expand Down
8 changes: 6 additions & 2 deletions fs/hfsplus/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
sb = dir->i_sb;

dentry->d_fsdata = NULL;
hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
if (err)
return ERR_PTR(err);
hfsplus_cat_build_key(sb, fd.search_key, dir->i_ino, &dentry->d_name);
again:
err = hfs_brec_read(&fd, &entry, sizeof(entry));
Expand Down Expand Up @@ -132,7 +134,9 @@ static int hfsplus_readdir(struct file *filp, void *dirent, filldir_t filldir)
if (filp->f_pos >= inode->i_size)
return 0;

hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
if (err)
return err;
hfsplus_cat_build_key(sb, fd.search_key, inode->i_ino, NULL);
err = hfs_brec_find(&fd);
if (err)
Expand Down
27 changes: 18 additions & 9 deletions fs/hfsplus/extents.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,10 @@ static void hfsplus_ext_write_extent_locked(struct inode *inode)
if (HFSPLUS_I(inode)->extent_state & HFSPLUS_EXT_DIRTY) {
struct hfs_find_data fd;

hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd);
__hfsplus_ext_write_extent(inode, &fd);
hfs_find_exit(&fd);
if (!hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd)) {
__hfsplus_ext_write_extent(inode, &fd);
hfs_find_exit(&fd);
}
}
}

Expand Down Expand Up @@ -194,9 +195,11 @@ static int hfsplus_ext_read_extent(struct inode *inode, u32 block)
block < hip->cached_start + hip->cached_blocks)
return 0;

hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd);
res = __hfsplus_ext_cache_extent(&fd, inode, block);
hfs_find_exit(&fd);
res = hfs_find_init(HFSPLUS_SB(inode->i_sb)->ext_tree, &fd);
if (!res) {
res = __hfsplus_ext_cache_extent(&fd, inode, block);
hfs_find_exit(&fd);
}
return res;
}

Expand Down Expand Up @@ -374,7 +377,9 @@ int hfsplus_free_fork(struct super_block *sb, u32 cnid,
if (total_blocks == blocks)
return 0;

hfs_find_init(HFSPLUS_SB(sb)->ext_tree, &fd);
res = hfs_find_init(HFSPLUS_SB(sb)->ext_tree, &fd);
if (res)
return res;
do {
res = __hfsplus_ext_read_extent(&fd, ext_entry, cnid,
total_blocks, type);
Expand Down Expand Up @@ -503,7 +508,6 @@ void hfsplus_file_truncate(struct inode *inode)
struct page *page;
void *fsdata;
u32 size = inode->i_size;
int res;

res = pagecache_write_begin(NULL, mapping, size, 0,
AOP_FLAG_UNINTERRUPTIBLE,
Expand All @@ -526,7 +530,12 @@ void hfsplus_file_truncate(struct inode *inode)
goto out;

mutex_lock(&hip->extents_lock);
hfs_find_init(HFSPLUS_SB(sb)->ext_tree, &fd);
res = hfs_find_init(HFSPLUS_SB(sb)->ext_tree, &fd);
if (res) {
mutex_unlock(&hip->extents_lock);
/* XXX: We lack error handling of hfsplus_file_truncate() */
return;
}
while (1) {
if (alloc_cnt == hip->first_blocks) {
hfsplus_free_extents(sb, hip->first_extents,
Expand Down
12 changes: 7 additions & 5 deletions fs/hfsplus/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,13 @@ static struct dentry *hfsplus_file_lookup(struct inode *dir,
hip->flags = 0;
set_bit(HFSPLUS_I_RSRC, &hip->flags);

hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
err = hfsplus_find_cat(sb, dir->i_ino, &fd);
if (!err)
err = hfsplus_cat_read_inode(inode, &fd);
hfs_find_exit(&fd);
err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
if (!err) {
err = hfsplus_find_cat(sb, dir->i_ino, &fd);
if (!err)
err = hfsplus_cat_read_inode(inode, &fd);
hfs_find_exit(&fd);
}
if (err) {
iput(inode);
return ERR_PTR(err);
Expand Down
16 changes: 10 additions & 6 deletions fs/hfsplus/super.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@ struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino)

if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||
inode->i_ino == HFSPLUS_ROOT_CNID) {
hfs_find_init(HFSPLUS_SB(inode->i_sb)->cat_tree, &fd);
err = hfsplus_find_cat(inode->i_sb, inode->i_ino, &fd);
if (!err)
err = hfsplus_cat_read_inode(inode, &fd);
hfs_find_exit(&fd);
err = hfs_find_init(HFSPLUS_SB(inode->i_sb)->cat_tree, &fd);
if (!err) {
err = hfsplus_find_cat(inode->i_sb, inode->i_ino, &fd);
if (!err)
err = hfsplus_cat_read_inode(inode, &fd);
hfs_find_exit(&fd);
}
} else {
err = hfsplus_system_read_inode(inode);
}
Expand Down Expand Up @@ -456,7 +458,9 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)

str.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
str.name = HFSP_HIDDENDIR_NAME;
hfs_find_init(sbi->cat_tree, &fd);
err = hfs_find_init(sbi->cat_tree, &fd);
if (err)
goto out_put_root;
hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
hfs_find_exit(&fd);
Expand Down

0 comments on commit 5bd9d99

Please sign in to comment.