Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5bd9d99d authored by Alexey Khoroshilov's avatar Alexey Khoroshilov Committed by Christoph Hellwig
Browse files

hfsplus: add error checking for hfs_find_init()



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: default avatarAlexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent c6d5f5fa
Loading
Loading
Loading
Loading
+10 −4
Original line number Original line Diff line number Diff line
@@ -212,7 +212,9 @@ int hfsplus_create_cat(u32 cnid, struct inode *dir,


	dprint(DBG_CAT_MOD, "create_cat: %s,%u(%d)\n",
	dprint(DBG_CAT_MOD, "create_cat: %s,%u(%d)\n",
		str->name, cnid, inode->i_nlink);
		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);
	hfsplus_cat_build_key(sb, fd.search_key, cnid, NULL);
	entry_size = hfsplus_fill_cat_thread(sb, &entry,
	entry_size = hfsplus_fill_cat_thread(sb, &entry,
@@ -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",
	dprint(DBG_CAT_MOD, "delete_cat: %s,%u\n",
		str ? str->name : NULL, cnid);
		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) {
	if (!str) {
		int len;
		int len;
@@ -347,12 +351,14 @@ int hfsplus_rename_cat(u32 cnid,
	struct hfs_find_data src_fd, dst_fd;
	struct hfs_find_data src_fd, dst_fd;
	hfsplus_cat_entry entry;
	hfsplus_cat_entry entry;
	int entry_size, type;
	int entry_size, type;
	int err = 0;
	int err;


	dprint(DBG_CAT_MOD, "rename_cat: %u - %lu,%s - %lu,%s\n",
	dprint(DBG_CAT_MOD, "rename_cat: %u - %lu,%s - %lu,%s\n",
		cnid, src_dir->i_ino, src_name->name,
		cnid, src_dir->i_ino, src_name->name,
		dst_dir->i_ino, dst_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;
	dst_fd = src_fd;


	/* find the old dir entry and read the data */
	/* find the old dir entry and read the data */
+6 −2
Original line number Original line Diff line number Diff line
@@ -38,7 +38,9 @@ static struct dentry *hfsplus_lookup(struct inode *dir, struct dentry *dentry,
	sb = dir->i_sb;
	sb = dir->i_sb;


	dentry->d_fsdata = NULL;
	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);
	hfsplus_cat_build_key(sb, fd.search_key, dir->i_ino, &dentry->d_name);
again:
again:
	err = hfs_brec_read(&fd, &entry, sizeof(entry));
	err = hfs_brec_read(&fd, &entry, sizeof(entry));
@@ -132,7 +134,9 @@ static int hfsplus_readdir(struct file *filp, void *dirent, filldir_t filldir)
	if (filp->f_pos >= inode->i_size)
	if (filp->f_pos >= inode->i_size)
		return 0;
		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);
	hfsplus_cat_build_key(sb, fd.search_key, inode->i_ino, NULL);
	err = hfs_brec_find(&fd);
	err = hfs_brec_find(&fd);
	if (err)
	if (err)
+18 −9
Original line number Original line Diff line number Diff line
@@ -124,11 +124,12 @@ static void hfsplus_ext_write_extent_locked(struct inode *inode)
	if (HFSPLUS_I(inode)->extent_state & HFSPLUS_EXT_DIRTY) {
	if (HFSPLUS_I(inode)->extent_state & HFSPLUS_EXT_DIRTY) {
		struct hfs_find_data fd;
		struct hfs_find_data fd;


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


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


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


@@ -374,7 +377,9 @@ int hfsplus_free_fork(struct super_block *sb, u32 cnid,
	if (total_blocks == blocks)
	if (total_blocks == blocks)
		return 0;
		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 {
	do {
		res = __hfsplus_ext_read_extent(&fd, ext_entry, cnid,
		res = __hfsplus_ext_read_extent(&fd, ext_entry, cnid,
						total_blocks, type);
						total_blocks, type);
@@ -503,7 +508,6 @@ void hfsplus_file_truncate(struct inode *inode)
		struct page *page;
		struct page *page;
		void *fsdata;
		void *fsdata;
		u32 size = inode->i_size;
		u32 size = inode->i_size;
		int res;


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


	mutex_lock(&hip->extents_lock);
	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) {
	while (1) {
		if (alloc_cnt == hip->first_blocks) {
		if (alloc_cnt == hip->first_blocks) {
			hfsplus_free_extents(sb, hip->first_extents,
			hfsplus_free_extents(sb, hip->first_extents,
+7 −5
Original line number Original line Diff line number Diff line
@@ -195,11 +195,13 @@ static struct dentry *hfsplus_file_lookup(struct inode *dir,
	hip->flags = 0;
	hip->flags = 0;
	set_bit(HFSPLUS_I_RSRC, &hip->flags);
	set_bit(HFSPLUS_I_RSRC, &hip->flags);


	hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
	err = hfs_find_init(HFSPLUS_SB(sb)->cat_tree, &fd);
	if (!err) {
		err = hfsplus_find_cat(sb, dir->i_ino, &fd);
		err = hfsplus_find_cat(sb, dir->i_ino, &fd);
		if (!err)
		if (!err)
			err = hfsplus_cat_read_inode(inode, &fd);
			err = hfsplus_cat_read_inode(inode, &fd);
		hfs_find_exit(&fd);
		hfs_find_exit(&fd);
	}
	if (err) {
	if (err) {
		iput(inode);
		iput(inode);
		return ERR_PTR(err);
		return ERR_PTR(err);
+10 −6
Original line number Original line Diff line number Diff line
@@ -73,11 +73,13 @@ struct inode *hfsplus_iget(struct super_block *sb, unsigned long ino)


	if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||
	if (inode->i_ino >= HFSPLUS_FIRSTUSER_CNID ||
	    inode->i_ino == HFSPLUS_ROOT_CNID) {
	    inode->i_ino == HFSPLUS_ROOT_CNID) {
		hfs_find_init(HFSPLUS_SB(inode->i_sb)->cat_tree, &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);
			err = hfsplus_find_cat(inode->i_sb, inode->i_ino, &fd);
			if (!err)
			if (!err)
				err = hfsplus_cat_read_inode(inode, &fd);
				err = hfsplus_cat_read_inode(inode, &fd);
			hfs_find_exit(&fd);
			hfs_find_exit(&fd);
		}
	} else {
	} else {
		err = hfsplus_system_read_inode(inode);
		err = hfsplus_system_read_inode(inode);
	}
	}
@@ -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.len = sizeof(HFSP_HIDDENDIR_NAME) - 1;
	str.name = HFSP_HIDDENDIR_NAME;
	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);
	hfsplus_cat_build_key(sb, fd.search_key, HFSPLUS_ROOT_CNID, &str);
	if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
	if (!hfs_brec_read(&fd, &entry, sizeof(entry))) {
		hfs_find_exit(&fd);
		hfs_find_exit(&fd);