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: <20141214210226.GA15238@birch.djwong.org>
Date:	Sun, 14 Dec 2014 13:02:26 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	"Theodore Ts'o" <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 11/47] libext2fs: find inode goal when allocating blocks

On Sat, Dec 13, 2014 at 08:13:27PM -0500, Theodore Ts'o wrote:
> 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-*

Oops, thank you for catching this one.  I periodically run make check with
USE_VALGRIND, forgot to do so. :/

--D

> 
> 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