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

Mingming Cao <[email protected]> Fri, 18 Aug 2006 10:38:10 -0700
Newsgroups gmane.comp.file-systems.ext2.devel
Message-ID <[email protected]>
Eric Sandeen wrote:
> 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
> 

Eric, the total number of blocks on the filesystem(s_blocks_count) is a 
  unsigned long long(sector_t) for ext4, so I suggest we use a unsigned 
long long type loop counters here, fix the overflow issue, and works for 
both ext3 and ext4:)

Mingming

> Signed-off-by: Eric Sandeen <[email protected]>
> 
> Index: e2fsprogs-1.39-test/e2fsck/pass4.c
> ===================================================================
> --- e2fsprogs-1.39-test.orig/e2fsck/pass4.c
> +++ e2fsprogs-1.39-test/e2fsck/pass4.c
> @@ -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) {
> Index: e2fsprogs-1.39-test/e2fsck/pass5.c
> ===================================================================
> --- e2fsprogs-1.39-test.orig/e2fsck/pass5.c
> +++ e2fsprogs-1.39-test/e2fsck/pass5.c
> @@ -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++)
> Index: e2fsprogs-1.39-test/lib/ext2fs/bitmaps.c
> ===================================================================
> --- e2fsprogs-1.39-test.orig/lib/ext2fs/bitmaps.c
> +++ e2fsprogs-1.39-test/lib/ext2fs/bitmaps.c
> @@ -102,7 +102,8 @@ 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;
> Index: e2fsprogs-1.39-test/resize/resize2fs.c
> ===================================================================
> --- e2fsprogs-1.39-test.orig/resize/resize2fs.c
> +++ e2fsprogs-1.39-test/resize/resize2fs.c
> @@ -1583,7 +1583,8 @@ 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
> _______________________________________________
> Ext2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ext2-devel


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