linux-ext4 - Re: [PATCH 11/47] libext2fs: find inode goal when allocating blocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141214011327.GA29787@thunk.org>
Date:	Sat, 13 Dec 2014 20:13:27 -0500
From:	Theodore Ts'o <tytso@....edu>
To:	"Darrick J. Wong" <darrick.wong@...cle.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 11/47] libext2fs: find inode goal when allocating blocks

On Fri, Nov 07, 2014 at 01:51:59PM -0800, Darrick J. Wong wrote:
> +	if (inode->i_flags & EXT4_EXTENTS_FL) {
> +		err = ext2fs_extent_open2(fs, ino, inode, &handle);
> +		if (err)
> +			goto no_blocks;
> +		err = ext2fs_extent_goto2(handle, 0, lblk);
> +		if (err)
> +			goto extent_err;
> +		err = ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent);
> +		if (err)
> +			goto extent_err;
> +		return extent.e_pblk + (lblk - extent.e_lblk);

There's a memory leak here; the extent handle never gets freed.  I've
checked this in with the attached fix up patched.

BTW, a really useful way of checking for memory leaks and which
allowed me to confirm the problem after noticing the problem from
reviewing the patch (and showing that with the attached patch below
the leak went away):

cd tests; ./test_script --valgrind-leakcheck f_extents f_extents2
more /tmp/valgrind-*

Cheers,

					- Ted

diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index 57bf5f0..62b36fe 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -296,7 +296,7 @@ blk64_t ext2fs_find_inode_goal(ext2_filsys fs, ext2_ino_t ino,
 	dgrp_t			group;
 	__u8			log_flex;
 	struct ext2fs_extent	extent;
-	ext2_extent_handle_t	handle;
+	ext2_extent_handle_t	handle = NULL;
 	errcode_t		err;
 
 	if (inode == NULL || ext2fs_inode_data_blocks2(fs, inode) == 0)
@@ -311,14 +311,12 @@ blk64_t ext2fs_find_inode_goal(ext2_filsys fs, ext2_ino_t ino,
 			goto no_blocks;
 		err = ext2fs_extent_goto2(handle, 0, lblk);
 		if (err)
-			goto extent_err;
+			goto no_blocks;
 		err = ext2fs_extent_get(handle, EXT2_EXTENT_CURRENT, &extent);
 		if (err)
-			goto extent_err;
-		return extent.e_pblk + (lblk - extent.e_lblk);
-extent_err:
+			goto no_blocks;
 		ext2fs_extent_free(handle);
-		goto no_blocks;
+		return extent.e_pblk + (lblk - extent.e_lblk);
 	}
 
 	/* block mapped file; see if block zero is mapped? */
@@ -326,6 +324,7 @@ extent_err:
 		return inode->i_block[0];
 
 no_blocks:
+	ext2fs_extent_free(handle);
 	log_flex = fs->super->s_log_groups_per_flex;
 	group = ext2fs_group_of_ino(fs, ino);
 	if (log_flex)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html