linux-ext4 - [PATCH 05/32] libext2fs: fix memory leak when drastically shrinking extent tree depth

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: <20140302071711.28217.49739.stgit@birch.djwong.org>
Date:	Sat, 01 Mar 2014 23:17:11 -0800
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	tytso@....edu, darrick.wong@...cle.com
Cc:	linux-ext4@...r.kernel.org
Subject: [PATCH 05/32] libext2fs: fix memory leak when drastically shrinking
 extent tree depth

In ext2fs_extent_free(), h(andle)->max_depth is used as a loop
conditional variable to free all the h->path[].buf pointers.  However,
ext2fs_extent_delete() sets max_depth = 0 if we've removed everything
from the extent tree, which causes a subsequent _free() to leak some
buf pointers.  max_depth can be re-incremented when splitting extent
nodes, but there's no guarantee that it'll reach the old value before
the free.

Therefore, remember the size of h->paths[] separately, and use that
when freeing the extent handle.

Signed-off-by: Darrick J. Wong <darrick.wong@...cle.com>
---
 lib/ext2fs/extent.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)


diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index 3ccae66..f27344e 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -58,6 +58,7 @@ struct ext2_extent_handle {
 	int			type;
 	int			level;
 	int			max_depth;
+	int			max_paths;
 	struct extent_path	*path;
 };
 
@@ -168,7 +169,7 @@ void ext2fs_extent_free(ext2_extent_handle_t handle)
 		return;
 
 	if (handle->path) {
-		for (i=1; i <= handle->max_depth; i++) {
+		for (i = 1; i < handle->max_paths; i++) {
 			if (handle->path[i].buf)
 				ext2fs_free_mem(&handle->path[i].buf);
 		}
@@ -242,11 +243,10 @@ errcode_t ext2fs_extent_open2(ext2_filsys fs, ext2_ino_t ino,
 	handle->max_depth = ext2fs_le16_to_cpu(eh->eh_depth);
 	handle->type = ext2fs_le16_to_cpu(eh->eh_magic);
 
-	retval = ext2fs_get_mem(((handle->max_depth+1) *
-				 sizeof(struct extent_path)),
-				&handle->path);
-	memset(handle->path, 0,
-	       (handle->max_depth+1) * sizeof(struct extent_path));
+	handle->max_paths = handle->max_depth + 1;
+	retval = ext2fs_get_memzero(handle->max_paths *
+				    sizeof(struct extent_path),
+				    &handle->path);
 	handle->path[0].buf = (char *) handle->inode->i_block;
 
 	handle->path[0].left = handle->path[0].entries =
@@ -912,13 +912,11 @@ errcode_t ext2fs_extent_node_split(ext2_extent_handle_t handle)
 	if (handle->level == 0) {
 		new_root = 1;
 		tocopy = ext2fs_le16_to_cpu(eh->eh_entries);
-		retval = ext2fs_get_mem(((handle->max_depth+2) *
-					 sizeof(struct extent_path)),
-					&newpath);
+		retval = ext2fs_get_memzero((handle->max_paths + 1) *
+					    sizeof(struct extent_path),
+					    &newpath);
 		if (retval)
 			goto done;
-		memset(newpath, 0,
-		       ((handle->max_depth+2) * sizeof(struct extent_path)));
 	} else {
 		tocopy = ext2fs_le16_to_cpu(eh->eh_entries) / 2;
 	}
@@ -996,13 +994,14 @@ errcode_t ext2fs_extent_node_split(ext2_extent_handle_t handle)
 	/* current path now has fewer active entries, we copied some out */
 	if (handle->level == 0) {
 		memcpy(newpath, path,
-		       sizeof(struct extent_path) * (handle->max_depth+1));
+		       sizeof(struct extent_path) * handle->max_paths);
 		handle->path = newpath;
 		newpath = path;
 		path = handle->path;
 		path->entries = 1;
 		path->left = path->max_entries - 1;
 		handle->max_depth++;
+		handle->max_paths++;
 		eh->eh_depth = ext2fs_cpu_to_le16(handle->max_depth);
 	} else {
 		path->entries -= tocopy;

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