diff mbox series

ext4: Fix crash when working on a fuzzed ext2 fs image

Message ID 20171207053818.23799-1-chandan@linux.vnet.ibm.com
State Accepted, archived
Headers show
Series ext4: Fix crash when working on a fuzzed ext2 fs image | expand

Commit Message

Chandan Rajendra Dec. 7, 2017, 5:38 a.m. UTC
On a ppc64 machine, when mounting a fuzzed ext2 image (generated by
fsfuzzer) the following call trace is seen,

VFS: brelse: Trying to free free buffer
WARNING: CPU: 1 PID: 6913 at /root/repos/linux/fs/buffer.c:1165 .__brelse.part.6+0x24/0x40
.__brelse.part.6+0x20/0x40 (unreliable)
.ext4_find_entry+0x384/0x4f0
.ext4_lookup+0x84/0x250
.lookup_slow+0xdc/0x230
.walk_component+0x268/0x400
.path_lookupat+0xec/0x2d0
.filename_lookup+0x9c/0x1d0
.vfs_statx+0x98/0x140
.SyS_newfstatat+0x48/0x80
system_call+0x58/0x6c

This happens because the directory that ext4_find_entry() looks up has
inode->i_size that is less than the block size of the filesystem. This
causes 'nblocks' to have a value of zero. ext4_bread_batch() ends up not
reading any of the directory file's blocks. This renders the entries in
bh_use[] array to continue to have garbage data. buffer_uptodate() on
bh_use[0] can then return a zero value upon which brelse() function is
invoked.

This commit fixes the bug by returning -ENOENT when the directory file
has no associated blocks.

Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
 fs/ext4/namei.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Theodore Ts'o Dec. 11, 2017, 8:09 p.m. UTC | #1
On Thu, Dec 07, 2017 at 11:08:18AM +0530, Chandan Rajendra wrote:
> On a ppc64 machine, when mounting a fuzzed ext2 image (generated by
> fsfuzzer) the following call trace is seen,
> ...
> 
> This happens because the directory that ext4_find_entry() looks up has
> inode->i_size that is less than the block size of the filesystem. This
> causes 'nblocks' to have a value of zero. ext4_bread_batch() ends up not
> reading any of the directory file's blocks. This renders the entries in
> bh_use[] array to continue to have garbage data. buffer_uptodate() on
> bh_use[0] can then return a zero value upon which brelse() function is
> invoked.

Thanks for reporting the problem.  Your patch wasn't quite right since
ret is not an int, but rather a struct buffer_head *, and the right
thing to do in this instnace is to return NULL:

>  	nblocks = dir->i_size >> EXT4_BLOCK_SIZE_BITS(sb);
> +	if (!nblocks) {
> +		ret = NULL;
> +		goto cleanup_and_exit;
> +	}


I'll fix up your patch and include it in the ext4 tree, thanks.

     	    	       	   	   - Ted
diff mbox series

Patch

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 798b3ac..42e2729 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1398,7 +1398,13 @@  static struct buffer_head * ext4_find_entry (struct inode *dir,
 		dxtrace(printk(KERN_DEBUG "ext4_find_entry: dx failed, "
 			       "falling back\n"));
 	}
+
 	nblocks = dir->i_size >> EXT4_BLOCK_SIZE_BITS(sb);
+	if (!nblocks) {
+		ret = -ENOENT;
+		goto cleanup_and_exit;
+	}
+
 	start = EXT4_I(dir)->i_dir_start_lookup;
 	if (start >= nblocks)
 		start = 0;