Commit c3d59ad6 authored by Carlos Maiolino's avatar Carlos Maiolino Committed by Jan Kara
Browse files

ext3: ext3_bread usage audit 



This is the ext3 version of the same patch applied to Ext4, where such goal is
to audit the usage of ext3_bread() due a possible misinterpretion of its return
value.

Focused on directory blocks, a NULL value returned from ext3_bread() means a
hole, which cannot exist into a directory inode. It can pass undetected after a
fix in an uninitialized error variable.

The (now) initialized variable into ext3_getblk() may lead to a zero'ed return
value of ext3_bread() to its callers, which can make the caller do not detect
the hole in the directory inode.

This patch creates a new wrapper function ext3_dir_bread() which checks for
holes properly, reports error, and returns EIO in that case.

Signed-off-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: default avatarJan Kara <jack@suse.cz>
parent aa966019

fs/ext3/namei.c

+20 −18
Original line number Diff line number Diff line
@@ -46,8 +46,7 @@ static struct buffer_head *ext3_append(handle_t *handle, 


	*block = inode->i_size >> inode->i_sb->s_blocksize_bits;



	bh = ext3_bread(handle, inode, *block, 1, err);

	if (bh) {

	if ((bh = ext3_dir_bread(handle, inode, *block, 1, err))) {

		inode->i_size += inode->i_sb->s_blocksize;

		EXT3_I(inode)->i_disksize = inode->i_size;

		*err = ext3_journal_get_write_access(handle, bh);
@@ -339,8 +338,10 @@ dx_probe(struct qstr *entry, struct inode *dir, 
	u32 hash;



	frame->bh = NULL;

	if (!(bh = ext3_bread (NULL,dir, 0, 0, err)))

	if (!(bh = ext3_dir_bread(NULL, dir, 0, 0, err))) {

		*err = ERR_BAD_DX_DIR;

		goto fail;

	}

	root = (struct dx_root *) bh->b_data;

	if (root->info.hash_version != DX_HASH_TEA &&

	    root->info.hash_version != DX_HASH_HALF_MD4 &&
@@ -436,8 +437,10 @@ dx_probe(struct qstr *entry, struct inode *dir, 
		frame->entries = entries;

		frame->at = at;

		if (!indirect--) return frame;

		if (!(bh = ext3_bread (NULL,dir, dx_get_block(at), 0, err)))

		if (!(bh = ext3_dir_bread(NULL, dir, dx_get_block(at), 0, err))) {

			*err = ERR_BAD_DX_DIR;

			goto fail2;

		}

		at = entries = ((struct dx_node *) bh->b_data)->entries;

		if (dx_get_limit(entries) != dx_node_limit (dir)) {

			ext3_warning(dir->i_sb, __func__,
@@ -535,7 +538,7 @@ static int ext3_htree_next_block(struct inode *dir, __u32 hash, 
	 * block so no check is necessary

	 */

	while (num_frames--) {

		if (!(bh = ext3_bread(NULL, dir, dx_get_block(p->at),

		if (!(bh = ext3_dir_bread(NULL, dir, dx_get_block(p->at),

					  0, &err)))

			return err; /* Failure */

		p++;
@@ -562,7 +565,8 @@ static int htree_dirblock_to_tree(struct file *dir_file, 
	int err = 0, count = 0;



	dxtrace(printk("In htree dirblock_to_tree: block %d\n", block));

	if (!(bh = ext3_bread (NULL, dir, block, 0, &err)))



	if (!(bh = ext3_dir_bread(NULL, dir, block, 0, &err)))

		return err;



	de = (struct ext3_dir_entry_2 *) bh->b_data;
@@ -976,7 +980,7 @@ static struct buffer_head * ext3_dx_find_entry(struct inode *dir, 
		return NULL;

	do {

		block = dx_get_block(frame->at);

		if (!(bh = ext3_bread (NULL,dir, block, 0, err)))

		if (!(bh = ext3_dir_bread (NULL, dir, block, 0, err)))

			goto errout;



		retval = search_dirblock(bh, dir, entry,
@@ -1458,9 +1462,9 @@ static int ext3_add_entry (handle_t *handle, struct dentry *dentry, 
	}

	blocks = dir->i_size >> sb->s_blocksize_bits;

	for (block = 0; block < blocks; block++) {

		bh = ext3_bread(handle, dir, block, 0, &retval);

		if(!bh)

		if (!(bh = ext3_dir_bread(handle, dir, block, 0, &retval)))

			return retval;



		retval = add_dirent_to_buf(handle, dentry, inode, NULL, bh);

		if (retval != -ENOSPC)

			return retval;
@@ -1500,7 +1504,7 @@ static int ext3_dx_add_entry(handle_t *handle, struct dentry *dentry, 
	entries = frame->entries;

	at = frame->at;



	if (!(bh = ext3_bread(handle,dir, dx_get_block(frame->at), 0, &err)))

	if (!(bh = ext3_dir_bread(handle, dir, dx_get_block(frame->at), 0, &err)))

		goto cleanup;



	BUFFER_TRACE(bh, "get_write_access");
@@ -1790,8 +1794,7 @@ retry: 
	inode->i_op = &ext3_dir_inode_operations;

	inode->i_fop = &ext3_dir_operations;

	inode->i_size = EXT3_I(inode)->i_disksize = inode->i_sb->s_blocksize;

	dir_block = ext3_bread (handle, inode, 0, 1, &err);

	if (!dir_block)

	if (!(dir_block = ext3_dir_bread(handle, inode, 0, 1, &err)))

		goto out_clear_inode;



	BUFFER_TRACE(dir_block, "get_write_access");
@@ -1859,7 +1862,7 @@ static int empty_dir (struct inode * inode) 


	sb = inode->i_sb;

	if (inode->i_size < EXT3_DIR_REC_LEN(1) + EXT3_DIR_REC_LEN(2) ||

	    !(bh = ext3_bread (NULL, inode, 0, 0, &err))) {

	    !(bh = ext3_dir_bread(NULL, inode, 0, 0, &err))) {

		if (err)

			ext3_error(inode->i_sb, __func__,

				   "error %d reading directory #%lu offset 0",
@@ -1890,9 +1893,8 @@ static int empty_dir (struct inode * inode) 
			(void *) de >= (void *) (bh->b_data+sb->s_blocksize)) {

			err = 0;

			brelse (bh);

			bh = ext3_bread (NULL, inode,

				offset >> EXT3_BLOCK_SIZE_BITS(sb), 0, &err);

			if (!bh) {

			if (!(bh = ext3_dir_bread (NULL, inode,

				offset >> EXT3_BLOCK_SIZE_BITS(sb), 0, &err))) {

				if (err)

					ext3_error(sb, __func__,

						   "error %d reading directory"
@@ -2388,7 +2390,7 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry, 
				goto end_rename;

		}

		retval = -EIO;

		dir_bh = ext3_bread (handle, old_inode, 0, 0, &retval);

		dir_bh = ext3_dir_bread(handle, old_inode, 0, 0, &retval);

		if (!dir_bh)

			goto end_rename;

		if (le32_to_cpu(PARENT_INO(dir_bh->b_data)) != old_dir->i_ino)

fs/ext3/namei.h

+19 −0
Original line number Diff line number Diff line
@@ -6,3 +6,22 @@ 
*/



extern struct dentry *ext3_get_parent(struct dentry *child);



static inline struct buffer_head *ext3_dir_bread(handle_t *handle,

						 struct inode *inode,

						 int block, int create,

						 int *err)

{

	struct buffer_head *bh;



	bh = ext3_bread(handle, inode, block, create, err);



	if (!bh && !(*err)) {

		*err = -EIO;

		ext3_error(inode->i_sb, __func__,

			   "Directory hole detected on inode %lu\n",

			   inode->i_ino);

		return NULL;

	}

	return bh;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE