linux-ext4 - Re: [PATCH 18/49] misc: fix header complaints and resource leaks in e2fsprogs

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: <20140314135350.GN8282@thunk.org>
Date:	Fri, 14 Mar 2014 09:53:50 -0400
From:	Theodore Ts'o <tytso@....edu>
To:	"Darrick J. Wong" <darrick.wong@...cle.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 18/49] misc: fix header complaints and resource leaks in
 e2fsprogs

On Mon, Mar 10, 2014 at 11:55:53PM -0700, Darrick J. Wong wrote:
> Fix a few minor bugs that cppcheck complained about.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@...cle.com>

Applied with the following changes.  It looks like cppcheck complained
with another false positive in ext2fs_create_icount_tdb().  The
filename is copied in icount->tdb_fn, and so adding a call to
ext2fs_free_mem() will actually result in a double-free bug, since
ext2fs_free_icount() will take care of releasing the memory.  Also,
perhaps just as importantly, it will take care of deleting the
temporary file created by mkstemp() first.

I did keep the first ext2fs_free_mem() and moved setting
icount->tdb_fn down by a bit just to avoid a potential bug if
mkstemp() fails, and there is a valid file of the form *-icount-XXXXXX
that the user would be unhappy with us deleting.  Pedantic, perhaps,
since it would probably never happen, but it's good to be 100%
correct.  :-)

					- Ted

diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 11c2693..b39383d 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -1016,7 +1016,6 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
 			strcat(newpath, oldpath);
 		}
 		putenv(newpath);
-		free(newpath);
 	}
 #ifdef CONFIG_JBD_DEBUG
 	jbd_debug = getenv("E2FSCK_JBD_DEBUG");
diff --git a/lib/ext2fs/icount.c b/lib/ext2fs/icount.c
index 7d1b3d5..5e1f5c6 100644
--- a/lib/ext2fs/icount.c
+++ b/lib/ext2fs/icount.c
@@ -193,7 +193,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir,
 		goto errout;
 	uuid_unparse(fs->super->s_uuid, uuid);
 	sprintf(fn, "%s/%s-icount-XXXXXX", tdb_dir, uuid);
-	icount->tdb_fn = fn;
 	save_umask = umask(077);
 	fd = mkstemp(fn);
 	if (fd < 0) {
@@ -201,6 +200,7 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir,
 		ext2fs_free_mem(&fn);
 		goto errout;
 	}
+	icount->tdb_fn = fn;
 	umask(save_umask);
 	/*
 	 * This is an overestimate of the size that we will need; the
@@ -217,7 +217,6 @@ errcode_t ext2fs_create_icount_tdb(ext2_filsys fs, char *tdb_dir,
 	close(fd);
 	if (icount->tdb == NULL) {
 		retval = errno;
-		ext2fs_free_mem(&fn);
 		goto errout;
 	}
 	*ret = icount;
--
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