Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] e2fsck: fix to return ENOMEM in alloc_size_dir()
@ 2019-11-26  9:03 Wang Shilong
  2019-11-26  9:03 ` [PATCH 2/2] e2fsck: fix use after free in calculate_tree() Wang Shilong
  0 siblings, 1 reply; 2+ messages in thread
From: Wang Shilong @ 2019-11-26  9:03 UTC (permalink / raw)
  To: linux-ext4; +Cc: adilger, lixi, wshilong

From: Wang Shilong <wshilong@ddn.com>

Two memory allocation return check is missed.

Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
 e2fsck/rehash.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
index a5fc1be1..5250652e 100644
--- a/e2fsck/rehash.c
+++ b/e2fsck/rehash.c
@@ -272,7 +272,11 @@ static errcode_t alloc_size_dir(ext2_filsys fs, struct out_dir *outdir,
 		outdir->hashes = new_mem;
 	} else {
 		outdir->buf = malloc(blocks * fs->blocksize);
+		if (!outdir->buf)
+			return ENOMEM;
 		outdir->hashes = malloc(blocks * sizeof(ext2_dirhash_t));
+		if (!outdir->hashes)
+			return ENOMEM;
 		outdir->num = 0;
 	}
 	outdir->max = blocks;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH 2/2] e2fsck: fix use after free in calculate_tree()
  2019-11-26  9:03 [PATCH 1/2] e2fsck: fix to return ENOMEM in alloc_size_dir() Wang Shilong
@ 2019-11-26  9:03 ` Wang Shilong
  0 siblings, 0 replies; 2+ messages in thread
From: Wang Shilong @ 2019-11-26  9:03 UTC (permalink / raw)
  To: linux-ext4; +Cc: adilger, lixi, wshilong

From: Wang Shilong <wshilong@ddn.com>

Hit following Seg errors randomly when running f_large_dir test:

+Signal (11) SIGSEGV si_code=SEGV_MAPERR fault addr=0x7f02cfffbc1a
+../e2fsck/e2fsck[0x43766e]
+/lib64/libpthread.so.0(+0xf7e0)[0x7f02d8c9a7e0]
+../e2fsck/e2fsck(e2fsck_rehash_dir+0x10f3)[0x436173]
+../e2fsck/e2fsck(e2fsck_rehash_directories+0xf4)[0x4362d4]
+../e2fsck/e2fsck(e2fsck_pass3+0x722)[0x4292c2]
+../e2fsck/e2fsck(e2fsck_run+0x47)[0x414ef7]
+../e2fsck/e2fsck(main+0x1c1d)[0x41319d]
+/lib64/libc.so.6(__libc_start_main+0x100)[0x7f02d8915d20]
+../e2fsck/e2fsck[0x40fc59]
+Exit status is 8

gdb output is:
0x436173 is in e2fsck_rehash_dir (rehash.c:752).
warning: Source file is more recent than executable.
747					dx_ent->hash =
748						ext2fs_cpu_to_le32(outdir->hashes[i]);
749				dx_ent++;
750				c3--;
751			}
752			int_limit->count = ext2fs_cpu_to_le16(limit->limit - c2);
753			int_limit->limit = ext2fs_cpu_to_le16(limit->limit);
754
755			limit->count = ext2fs_cpu_to_le16(limit->limit - c3);
756			limit->limit = ext2fs_cpu_to_le16(limit->limit);

The problem is alloc_blocks() will call get_next_block()
which might reallocate @outdir->buf, and memory address
could be changed after this. @int_limit and @root should
be recalculated based on new start address. Otherwise,
it will try to access freed memory and cause SEGV_MAPERR
errors.

Signed-off-by: Wang Shilong <wshilong@ddn.com>
---
 e2fsck/rehash.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
index 5250652e..0eb99328 100644
--- a/e2fsck/rehash.c
+++ b/e2fsck/rehash.c
@@ -636,6 +636,9 @@ static int alloc_blocks(ext2_filsys fs,
 	if (retval)
 		return retval;
 
+	/* outdir->buf might be reallocated */
+	*prev_ent = (struct ext2_dx_entry *) (outdir->buf + *prev_offset);
+
 	*next_ent = set_int_node(fs, block_start);
 	*limit = (struct ext2_dx_countlimit *)(*next_ent);
 	if (next_offset)
@@ -725,12 +728,18 @@ static errcode_t calculate_tree(ext2_filsys fs,
 					return retval;
 			}
 			if (c3 == 0) {
+				int delta1 = int_offset;;
+				int delta2 = (char *)root - outdir->buf;
+
 				retval = alloc_blocks(fs, &limit, &int_ent,
 						      &dx_ent, &int_offset,
 						      NULL, outdir, i, &c2,
 						      &c3);
 				if (retval)
 					return retval;
+				/* outdir->buf might be reallocated */
+				int_limit = (struct ext2_dx_countlimit *)(outdir->buf + delta1);
+				root = (struct ext2_dx_entry *)(outdir->buf + delta2);
 
 			}
 			dx_ent->block = ext2fs_cpu_to_le32(i);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  9:03 [PATCH 1/2] e2fsck: fix to return ENOMEM in alloc_size_dir() Wang Shilong
2019-11-26  9:03 ` [PATCH 2/2] e2fsck: fix use after free in calculate_tree() Wang Shilong

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git