Re: [PATCH] 2/6 Detect overflows in loop counters

Theodore Tso <[email protected]> Wed, 30 Aug 2006 02:18:25 -0400
Newsgroups gmane.comp.file-systems.ext2.devel
Message-ID <[email protected]>
Thanks, applied.

As a reminder, I'd appreciate it if you included ChangeLog entries and
if you try to avoid lines in C code which overrun 80 columns.  I fixed
those up after I applied your patch.

Regards,

						- Ted

# HG changeset patch
# User [email protected]
# Date 1156918615 14400
# Node ID 14e45223b10be14cc318f10b804a3fd535a86ad5
# Parent  d609388faa895de79ff143e53f8ed04557048c42
Detect overflows in loop counters

For loops such as:

for (i=1; i <= fs->super->s_blocks_count; i++) {
        <do_stuff>
}

if i is an int and s_blocks_count is (2^32-1), the condition is never false.
Change these loops to:

for (i=1; i <= fs->super->s_blocks_count && i > 0; i++) {
        <do_stuff>
}

to stop the loop when we overflow i

Signed-off-by: Eric Sandeen <[email protected]>
Signed-off-by: "Theodore Ts'o" <[email protected]>

diff -r d609388faa89 -r 14e45223b10b e2fsck/ChangeLog
--- a/e2fsck/ChangeLog	Wed Aug 30 02:06:41 2006 -0400
+++ b/e2fsck/ChangeLog	Wed Aug 30 02:16:55 2006 -0400
@@ -1,3 +1,9 @@ 2006-08-29  Theodore Tso  <[email protected]
+2006-08-30  Theodore Tso  <[email protected]>
+
+	* pass5.c (check_inode_bitmaps, check_inode_end, check_block_end):
+	* pass4.c (e2fsck_pass4): Fix potential overflow problems when the
+		number of blocks is close to 2**31.
+
 2006-08-29  Theodore Tso  <[email protected]>
 
 	* super.c (release_inode_blocks): Fix silly spelling error.
diff -r d609388faa89 -r 14e45223b10b e2fsck/pass4.c
--- a/e2fsck/pass4.c	Wed Aug 30 02:06:41 2006 -0400
+++ b/e2fsck/pass4.c	Wed Aug 30 02:16:55 2006 -0400
@@ -110,8 +110,9 @@ void e2fsck_pass4(e2fsck_t ctx)
 	if (ctx->progress)
 		if ((ctx->progress)(ctx, 4, 0, maxgroup))
 			return;
-	
-	for (i=1; i <= fs->super->s_inodes_count; i++) {
+
+	/* Protect loop from wrap-around if s_inodes_count maxed */
+	for (i=1; i <= fs->super->s_inodes_count && i > 0; i++) {
 		if (ctx->flags & E2F_FLAG_SIGNAL_MASK)
 			return;
 		if ((i % fs->super->s_inodes_per_group) == 0) {
diff -r d609388faa89 -r 14e45223b10b e2fsck/pass5.c
--- a/e2fsck/pass5.c	Wed Aug 30 02:06:41 2006 -0400
+++ b/e2fsck/pass5.c	Wed Aug 30 02:16:55 2006 -0400
@@ -370,7 +370,8 @@ redo_counts:
 			EXT2_BG_INODE_UNINIT))
 		skip_group++;
 
-	for (i = 1; i <= fs->super->s_inodes_count; i++) {
+	/* Protect loop from wrap-around if inodes_count is maxed */
+	for (i = 1; i <= fs->super->s_inodes_count && i > 0; i++) {
 		actual = ext2fs_fast_test_inode_bitmap(ctx->inode_used_map, i);
 		if (skip_group) 
 			bitmap = 0;
@@ -528,8 +529,9 @@ static void check_inode_end(e2fsck_t ctx
 	}
 	if (save_inodes_count == end)
 		return;
-	
-	for (i = save_inodes_count + 1; i <= end; i++) {
+
+	/* protect loop from wrap-around if end is maxed */	
+	for (i = save_inodes_count + 1; i <= end && i > save_inodes_count; i++) {
 		if (!ext2fs_test_inode_bitmap(fs->inode_map, i)) {
 			if (fix_problem(ctx, PR_5_INODE_BMAP_PADDING, &pctx)) {
 				for (i = save_inodes_count + 1; i <= end; i++)
@@ -572,8 +574,9 @@ static void check_block_end(e2fsck_t ctx
 	}
 	if (save_blocks_count == end)
 		return;
-	
-	for (i = save_blocks_count + 1; i <= end; i++) {
+
+	/* Protect loop from wrap-around if end is maxed */	
+	for (i = save_blocks_count + 1; i <= end && i > save_blocks_count; i++) {
 		if (!ext2fs_test_block_bitmap(fs->block_map, i)) {
 			if (fix_problem(ctx, PR_5_BLOCK_BMAP_PADDING, &pctx)) {
 				for (i = save_blocks_count + 1; i <= end; i++)
diff -r d609388faa89 -r 14e45223b10b lib/ext2fs/ChangeLog
--- a/lib/ext2fs/ChangeLog	Wed Aug 30 02:06:41 2006 -0400
+++ b/lib/ext2fs/ChangeLog	Wed Aug 30 02:16:55 2006 -0400
@@ -1,4 +1,7 @@ 2006-08-30  Theodore Tso  <[email protected]
 2006-08-30  Theodore Tso  <[email protected]>
+
+	* bitmaps.c (ext2fs_set_bitmap_padding): Fix potential overflow
+		problems when the number of blocks is close to 2**31.
 
 	* ext2fs.h (ext2fs_div_ceil): Add new function which safely
 		calculates an integer division where the result is always
diff -r d609388faa89 -r 14e45223b10b lib/ext2fs/bitmaps.c
--- a/lib/ext2fs/bitmaps.c	Wed Aug 30 02:06:41 2006 -0400
+++ b/lib/ext2fs/bitmaps.c	Wed Aug 30 02:16:55 2006 -0400
@@ -102,7 +102,10 @@ void ext2fs_set_bitmap_padding(ext2fs_ge
 {
 	__u32	i, j;
 
-	for (i=map->end+1, j = i - map->start; i <= map->real_end; i++, j++)
+	/* Protect loop from wrap-around if map->real_end is maxed */
+	for (i=map->end+1, j = i - map->start; 
+	     i <= map->real_end && i > map->end; 
+	     i++, j++)
 		ext2fs_set_bit(j, map->bitmap);
 
 	return;
diff -r d609388faa89 -r 14e45223b10b resize/ChangeLog
--- a/resize/ChangeLog	Wed Aug 30 02:06:41 2006 -0400
+++ b/resize/ChangeLog	Wed Aug 30 02:16:55 2006 -0400
@@ -1,4 +1,8 @@ 2006-08-30  Theodore Tso  <[email protected]
 2006-08-30  Theodore Tso  <[email protected]>
+
+	* resize2fs.c (ext2fs_calculate_summary_stats): Fix potential
+		overflow problems when the number of blocks is close to
+		2**31.
 
 	* resize2fs.c (adjust_fs_info): Use ext2fs_div_ceil() instead of a
 		using an open-coded expression which was subject to
diff -r d609388faa89 -r 14e45223b10b resize/resize2fs.c
--- a/resize/resize2fs.c	Wed Aug 30 02:06:41 2006 -0400
+++ b/resize/resize2fs.c	Wed Aug 30 02:16:55 2006 -0400
@@ -1582,7 +1582,9 @@ static errcode_t ext2fs_calculate_summar
 	total_free = 0;
 	count = 0;
 	group = 0;
-	for (ino = 1; ino <= fs->super->s_inodes_count; ino++) {
+
+	/* Protect loop from wrap-around if s_inodes_count maxed */
+	for (ino = 1; ino <= fs->super->s_inodes_count && ino > 0; ino++) {
 		if (!ext2fs_fast_test_inode_bitmap(fs->inode_map, ino)) {
 			group_free++;
 			total_free++;

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642