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: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/hfsplus/catalog.c b/fs/hfsplus/catalog.c
index b4ba1b3193336ead42c6ffbfbbd365e058cf8f1e..4dfbfec357e8837055c2131206e0ca812fbec4de 100644 (file)
--- a/fs/hfsplus/catalog.c
+++ b/fs/hfsplus/catalog.c
@@ -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,
@@ -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;
@@ -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 */

diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index 4df5059c25da67c4b1ddd2a868057951e6e54d60..25b2443a004cd070ab49fe94e167898d32271a56 100644 (file)
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -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));
@@ -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)

diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index b9c1a4b5ba89db557c604631c9a8d7f286b1d4d9..95065a89e7e23de93d39210584066e03fdb112dc 100644 (file)
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -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);
+               }
        }
 }
 
@@ -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;
 }
 
@@ -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);
@@ -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,
@@ -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,

diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index b248a6cfcad93bb76631f7756b07b4ee6aff3d7e..010cd363d085d8d4fd43b3e677451a1192a1c91f 100644 (file)
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -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);

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index acaef57e3606165cf096e13f8fac3566e6153871..2c1a72287fb5fd18429c24dad86b271792c3aa62 100644 (file)
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -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);
        }
@@ -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);