[f2fs-dev] [PATCH 1/3] fsck.f2fs: fix to avoid memory leak reported by LeakSanitizer

[f2fs-dev] [PATCH 1/3] fsck.f2fs: fix to avoid memory leak reported by LeakSanitizer

[Chao Yu via Linux-f2fs-devel]

==5957==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7f08b78bc340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x55b9e0fcbcc7 in update_data_blkaddr /share/tools-f2fs/fsck/mount.c:2523

SUMMARY: AddressSanitizer: 4096 byte(s) leaked in 1 allocation(s).

When a process calls exit(), the C runtime executes all registered
atexit handlers.
AddressSanitizer / LeakSanitizer (LSan) hooks into exit() to inspect
the heap, it checks whether all memory allocated via malloc() / calloc()
was explicitly released.

Relocate sanity check and ASSERT(0) before node block allocation to fix
this issue.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fsck/mount.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index 2e7ea45..26f213e 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -2520,22 +2520,22 @@ void update_data_blkaddr(struct f2fs_sb_info *sbi, nid_t nid,
 	if (node_blk == NULL) {
 		struct seg_entry *se;
 
-		node_blk = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
-		ASSERT(node_blk);
-
 		get_node_info(sbi, nid, &ni);
 
-		/* read node_block */
-		ret = dev_read_block(node_blk, ni.blk_addr);
-		ASSERT(ret >= 0);
-		node_blk_alloced = true;
-
 		se = get_seg_entry(sbi, GET_SEGNO(sbi, ni.blk_addr));
 		if (IS_DATASEG(se->type)) {
 			ERR_MSG("NAT and SIT is inconsistent: ino: %u, nid: %u, blkaddr: %u, segtype: %d",
 				ni.ino, ni.nid, ni.blk_addr, se->type);
 			ASSERT(0);
 		}
+
+		node_blk = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
+		ASSERT(node_blk);
+
+		/* read node_block */
+		ret = dev_read_block(node_blk, ni.blk_addr);
+		ASSERT(ret >= 0);
+		node_blk_alloced = true;
 	}
 
 	/* check its block address */
-- 
2.49.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 2/3] fsck.f2fs: sanity check i_extra_isize correctly

[Chao Yu via Linux-f2fs-devel]

In fsck_chk_inode_blk(), fsck.f2fs checks whether i_extra_isize is
larger than 4 * DEF_ADDRS_PER_INODE. However, this upper threshold is
too loose and fails to check the lower boundary or 4-byte alignment.

When an inode has a corrupted i_extra_isize (e.g. 3692), it bypasses
the sanity check. Subsequent calculation in addrs_per_page() causes an
unsigned integer underflow (CUR_ADDRS_PER_INODE - get_inline_xattr_addrs),
resulting in a huge loop bound in fsck_chk_inode_blk() and out-of-bounds
heap reads and writes.

This patch fixes the issue by:
1. Defining F2FS_MIN_EXTRA_ATTR_SIZE in include/f2fs_fs.h.
2. Checking i_extra_isize against F2FS_MIN_EXTRA_ATTR_SIZE,
   F2FS_TOTAL_EXTRA_ATTR_SIZE, and 4-byte alignment, matching the
   kernel-side sanity_check_inode() logic.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fsck/fsck.c       | 4 +++-
 include/f2fs_fs.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 06bf721..6872b00 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1038,7 +1038,9 @@ check_next:
 			unsigned int isize =
 				le16_to_cpu(node_blk->i.i_extra_isize);
 			if (time_to_inject(FAULT_INODE) ||
-					(isize > 4 * DEF_ADDRS_PER_INODE)) {
+					(isize < F2FS_MIN_EXTRA_ATTR_SIZE) ||
+					(isize > F2FS_TOTAL_EXTRA_ATTR_SIZE) ||
+					(isize % sizeof(__le32))) {
 				ASSERT_MSG("[0x%x] wrong i_extra_isize=0x%x",
 						nid, isize);
 				if (c.fix_on) {
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 432c80c..4225ffe 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -979,6 +979,7 @@ static_assert(sizeof(struct node_footer) == 24, "");
 
 #define F2FS_EXTRA_ISIZE_OFFSET				\
 	offsetof(struct f2fs_inode, i_extra_isize)
+#define F2FS_MIN_EXTRA_ATTR_SIZE	(sizeof(__le32))
 #define F2FS_TOTAL_EXTRA_ATTR_SIZE			\
 	(offsetof(struct f2fs_inode, i_extra_end) - F2FS_EXTRA_ISIZE_OFFSET)
 
-- 
2.49.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH 3/3] fsck.f2fs: sanity check quota file size in v2_init_io

[Chao Yu via Linux-f2fs-devel]

In v2_init_io(), quota file size is not bounded. A corrupted or crafted
quota inode with a large i_size can set dqi_blocks to a huge value
(e.g. 0xFFFFFFFC).

When scanning dquots in qtree_scan_dquots(), computing the bitmap size
as (info->dqi_blocks + 7) >> 3 causes an unsigned 32-bit integer wrap to 0,
allocating a 0-byte buffer via malloc(0). Later, report_block() performs
out-of-bounds bit-setting writes past the heap buffer.

This patch fixes the issue by:
1. Porting the e2fsprogs check (CVE-2019-5094) into v2_init_io() to cap
   quota file size at 2GB (1ULL << 31).
2. Casting info->dqi_blocks to size_t when calculating bitmap allocation
   size in qtree_scan_dquots() as defense-in-depth.

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fsck/quotaio_tree.c | 2 +-
 fsck/quotaio_v2.c   | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fsck/quotaio_tree.c b/fsck/quotaio_tree.c
index 40521c5..60cec7d 100644
--- a/fsck/quotaio_tree.c
+++ b/fsck/quotaio_tree.c
@@ -667,7 +667,7 @@ int qtree_scan_dquots(struct quota_handle *h,
 		return -1;
 
 	dquot->dq_h = h;
-	if (quota_get_memzero((info->dqi_blocks + 7) >> 3, &bitmap))
+	if (quota_get_memzero(((size_t)info->dqi_blocks + 7) >> 3, &bitmap))
 		goto out;
 	if (report_tree(dquot, QT_TREEOFF, 0, bitmap, &entries, process_dquot,
 				data))
diff --git a/fsck/quotaio_v2.c b/fsck/quotaio_v2.c
index e19f303..50daf26 100644
--- a/fsck/quotaio_v2.c
+++ b/fsck/quotaio_v2.c
@@ -206,6 +206,12 @@ static int v2_init_io(struct quota_handle *h, enum quota_type qtype)
 		f2fs_filesize_update(qf->sbi, qf->ino, filesize);
 	}
 
+	if (filesize > (1ULL << 31)) {
+		log_err("Quota inode %u corrupted: file size %" PRIu64
+			" too large", h->qh_qf.ino, filesize);
+		return -1;
+	}
+
 	if ((info->dqi_qtree.dqi_blocks >
 			(filesize + QT_BLKSIZE - 1) >> QT_BLKSIZE_BITS)) {
 		log_err("Quota inode %u corrupted: file size %" PRId64 "; "
-- 
2.49.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel