Skip to content

Commit

Permalink
hfs: 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 <[email protected]>
Reviewed-by: Vyacheslav Dubeyko <[email protected]>
Cc: Hin-Tak Leung <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Artem Bityutskiy <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
@khoroshilov @torvalds
khoroshilov authored and torvalds committed May 1, 2013
1 parent eb53b6d commit 9509f17
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 24 deletions.
12 changes: 9 additions & 3 deletions fs/hfs/catalog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ int hfs_cat_create(u32 cnid, struct inode *dir, struct qstr *str, struct inode *
return -ENOSPC;

sb = dir->i_sb;
hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
if (err)
return err;

hfs_cat_build_key(sb, fd.search_key, cnid, NULL);
entry_size = hfs_cat_build_thread(sb, &entry, S_ISDIR(inode->i_mode) ?
Expand Down Expand Up @@ -214,7 +216,9 @@ int hfs_cat_delete(u32 cnid, struct inode *dir, struct qstr *str)

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

hfs_cat_build_key(sb, fd.search_key, dir->i_ino, str);
res = hfs_brec_find(&fd);
Expand Down Expand Up @@ -281,7 +285,9 @@ int hfs_cat_move(u32 cnid, struct inode *src_dir, struct qstr *src_name,
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);
sb = src_dir->i_sb;
hfs_find_init(HFS_SB(sb)->cat_tree, &src_fd);
err = hfs_find_init(HFS_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/hfs/dir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ static struct dentry *hfs_lookup(struct inode *dir, struct dentry *dentry,
struct inode *inode = NULL;
int res;

hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
res = hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
if (res)
return ERR_PTR(res);
hfs_cat_build_key(dir->i_sb, fd.search_key, dir->i_ino, &dentry->d_name);
res = hfs_brec_read(&fd, &rec, sizeof(rec));
if (res) {
Expand Down Expand Up @@ -63,7 +65,9 @@ static int hfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
if (filp->f_pos >= inode->i_size)
return 0;

hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
err = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
if (err)
return err;
hfs_cat_build_key(sb, fd.search_key, inode->i_ino, NULL);
err = hfs_brec_find(&fd);
if (err)
Expand Down
48 changes: 33 additions & 15 deletions fs/hfs/extent.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ static u16 hfs_ext_lastblock(struct hfs_extent *ext)
return be16_to_cpu(ext->block) + be16_to_cpu(ext->count);
}

static void __hfs_ext_write_extent(struct inode *inode, struct hfs_find_data *fd)
static int __hfs_ext_write_extent(struct inode *inode, struct hfs_find_data *fd)
{
int res;

Expand All @@ -116,26 +116,31 @@ static void __hfs_ext_write_extent(struct inode *inode, struct hfs_find_data *fd
res = hfs_brec_find(fd);
if (HFS_I(inode)->flags & HFS_FLG_EXT_NEW) {
if (res != -ENOENT)
return;
return res;
hfs_brec_insert(fd, HFS_I(inode)->cached_extents, sizeof(hfs_extent_rec));
HFS_I(inode)->flags &= ~(HFS_FLG_EXT_DIRTY|HFS_FLG_EXT_NEW);
} else {
if (res)
return;
return res;
hfs_bnode_write(fd->bnode, HFS_I(inode)->cached_extents, fd->entryoffset, fd->entrylength);
HFS_I(inode)->flags &= ~HFS_FLG_EXT_DIRTY;
}
return 0;
}

void hfs_ext_write_extent(struct inode *inode)
int hfs_ext_write_extent(struct inode *inode)
{
struct hfs_find_data fd;
int res = 0;

if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY) {
hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
__hfs_ext_write_extent(inode, &fd);
res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
if (res)
return res;
res = __hfs_ext_write_extent(inode, &fd);
hfs_find_exit(&fd);
}
return res;
}

static inline int __hfs_ext_read_extent(struct hfs_find_data *fd, struct hfs_extent *extent,
Expand All @@ -161,8 +166,11 @@ static inline int __hfs_ext_cache_extent(struct hfs_find_data *fd, struct inode
{
int res;

if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY)
__hfs_ext_write_extent(inode, fd);
if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY) {
res = __hfs_ext_write_extent(inode, fd);
if (res)
return res;
}

res = __hfs_ext_read_extent(fd, HFS_I(inode)->cached_extents, inode->i_ino,
block, HFS_IS_RSRC(inode) ? HFS_FK_RSRC : HFS_FK_DATA);
Expand All @@ -185,9 +193,11 @@ static int hfs_ext_read_extent(struct inode *inode, u16 block)
block < HFS_I(inode)->cached_start + HFS_I(inode)->cached_blocks)
return 0;

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

Expand Down Expand Up @@ -298,7 +308,9 @@ int hfs_free_fork(struct super_block *sb, struct hfs_cat_file *file, int type)
if (total_blocks == blocks)
return 0;

hfs_find_init(HFS_SB(sb)->ext_tree, &fd);
res = hfs_find_init(HFS_SB(sb)->ext_tree, &fd);
if (res)
return res;
do {
res = __hfs_ext_read_extent(&fd, extent, cnid, total_blocks, type);
if (res)
Expand Down Expand Up @@ -438,7 +450,9 @@ int hfs_extend_file(struct inode *inode)

insert_extent:
dprint(DBG_EXTENT, "insert new extent\n");
hfs_ext_write_extent(inode);
res = hfs_ext_write_extent(inode);
if (res)
goto out;

memset(HFS_I(inode)->cached_extents, 0, sizeof(hfs_extent_rec));
HFS_I(inode)->cached_extents[0].block = cpu_to_be16(start);
Expand Down Expand Up @@ -466,7 +480,6 @@ void hfs_file_truncate(struct inode *inode)
struct address_space *mapping = inode->i_mapping;
void *fsdata;
struct page *page;
int res;

/* XXX: Can use generic_cont_expand? */
size = inode->i_size - 1;
Expand All @@ -488,7 +501,12 @@ void hfs_file_truncate(struct inode *inode)
goto out;

mutex_lock(&HFS_I(inode)->extents_lock);
hfs_find_init(HFS_SB(sb)->ext_tree, &fd);
res = hfs_find_init(HFS_SB(sb)->ext_tree, &fd);
if (res) {
mutex_unlock(&HFS_I(inode)->extents_lock);
/* XXX: We lack error handling of hfs_file_truncate() */
return;
}
while (1) {
if (alloc_cnt == HFS_I(inode)->first_blocks) {
hfs_free_extents(sb, HFS_I(inode)->first_extents,
Expand Down
2 changes: 1 addition & 1 deletion fs/hfs/hfs_fs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ extern const struct inode_operations hfs_dir_inode_operations;
/* extent.c */
extern int hfs_ext_keycmp(const btree_key *, const btree_key *);
extern int hfs_free_fork(struct super_block *, struct hfs_cat_file *, int);
extern void hfs_ext_write_extent(struct inode *);
extern int hfs_ext_write_extent(struct inode *);
extern int hfs_extend_file(struct inode *);
extern void hfs_file_truncate(struct inode *);

Expand Down
11 changes: 9 additions & 2 deletions fs/hfs/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,12 @@ int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
struct inode *main_inode = inode;
struct hfs_find_data fd;
hfs_cat_rec rec;
int res;

dprint(DBG_INODE, "hfs_write_inode: %lu\n", inode->i_ino);
hfs_ext_write_extent(inode);
res = hfs_ext_write_extent(inode);
if (res)
return res;

if (inode->i_ino < HFS_FIRSTUSER_CNID) {
switch (inode->i_ino) {
Expand Down Expand Up @@ -515,7 +518,11 @@ static struct dentry *hfs_file_lookup(struct inode *dir, struct dentry *dentry,
if (!inode)
return ERR_PTR(-ENOMEM);

hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
res = hfs_find_init(HFS_SB(dir->i_sb)->cat_tree, &fd);
if (res) {
iput(inode);
return ERR_PTR(res);
}
fd.search_key->cat = HFS_I(dir)->cat_key;
res = hfs_brec_read(&fd, &rec, sizeof(rec));
if (!res) {
Expand Down
4 changes: 3 additions & 1 deletion fs/hfs/super.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,9 @@ static int hfs_fill_super(struct super_block *sb, void *data, int silent)
}

/* try to get the root inode */
hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
res = hfs_find_init(HFS_SB(sb)->cat_tree, &fd);
if (res)
goto bail_no_root;
res = hfs_cat_find_brec(sb, HFS_ROOT_CNID, &fd);
if (!res) {
if (fd.entrylength > sizeof(rec) || fd.entrylength < 0) {
Expand Down

0 comments on commit 9509f17

Please sign in to comment.