[PATCH v2 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

[PATCH v2 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

[Xiaoguang Wang]

When we use ext2fs_open_inode_scan() to iterate inodes and finish
jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
inode_scan_and_fix(), we forgot to call it, fix this error.

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
---
 misc/tune2fs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 7fee870..065b483 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2147,6 +2147,7 @@ static int inode_scan_and_fix(ext2_filsys fs, ext2fs_block_bitmap bmap)
 
 err_out:
 	ext2fs_free_mem(&block_buf);
+	ext2fs_close_inode_scan(scan);
 
 	return retval;
 }
-- 
1.8.2.1

[PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Xiaoguang Wang]

When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
obviously, we need to re-compute the group descriptor checksums, fix this. If
not doing this, mount operation will fail.

Meanwhile, the patch will trigger an existing memory write overflow, which will
casue segfault, please see the next patch.

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
---
 misc/tune2fs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 065b483..91dc7c1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2908,8 +2908,7 @@ retry_open:
 				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
 			rewrite_checksums = 1;
 	}
-	if (rewrite_checksums)
-		rewrite_metadata_checksums(fs);
+
 	if (I_flag) {
 		if (mount_flags & EXT2_MF_MOUNTED) {
 			fputs(_("The inode size may only be "
@@ -2935,6 +2934,7 @@ retry_open:
 		if (resize_inode(fs, new_inode_size) == 0) {
 			printf(_("Setting inode size %lu\n"),
 							new_inode_size);
+			rewrite_checksums = 1;
 		} else {
 			printf("%s", _("Failed to change inode size\n"));
 			rc = 1;
@@ -2942,6 +2942,9 @@ retry_open:
 		}
 	}
 
+	if (rewrite_checksums)
+		rewrite_metadata_checksums(fs);
+
 	if (l_flag)
 		list_super(sb);
 	if (stride_set) {
-- 
1.8.2.1

Re: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Darrick J. Wong]

On Wed, Nov 12, 2014 at 05:49:39PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, fix this. If
> not doing this, mount operation will fail.
> 
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> ---
>  misc/tune2fs.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 065b483..91dc7c1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2908,8 +2908,7 @@ retry_open:
>  				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>  			rewrite_checksums = 1;
>  	}
> -	if (rewrite_checksums)
> -		rewrite_metadata_checksums(fs);
> +
>  	if (I_flag) {
>  		if (mount_flags & EXT2_MF_MOUNTED) {
>  			fputs(_("The inode size may only be "
> @@ -2935,6 +2934,7 @@ retry_open:
>  		if (resize_inode(fs, new_inode_size) == 0) {
>  			printf(_("Setting inode size %lu\n"),
>  							new_inode_size);
> +			rewrite_checksums = 1;
>  		} else {
>  			printf("%s", _("Failed to change inode size\n"));
>  			rc = 1;
> @@ -2942,6 +2942,9 @@ retry_open:
>  		}
>  	}
>  
> +	if (rewrite_checksums)
> +		rewrite_metadata_checksums(fs);
> +

Aha!  expand_inode_table() fails to recompute the checksums of the inode blocks
it's moving around, and happily your change takes care of recomputing the inode
checksums for a metadata_csum FS.  The changelog for this patch doesn't mention
this, but it should.

There ought to be a regression test for this.  Can you send one along, please?

I crafted my own test case for the metadata_csum failure while trying to figure
out what this patchset does, so I'll simply send it out.  Unfortunately, the
test requires a fix for a bug in the hugefile code that I'll cram onto the
patchbomb.

Other than that, you can add for all three patches:
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>  	if (l_flag)
>  		list_super(sb);
>  	if (stride_set) {
> -- 
> 1.8.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Xiaoguang Wang]

Hi,
On 11/13/2014 06:39 AM, Darrick J. Wong wrote:
> On Wed, Nov 12, 2014 at 05:49:39PM +0800, Xiaoguang Wang wrote:
>> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
>> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
>> obviously, we need to re-compute the group descriptor checksums, fix this. If
>> not doing this, mount operation will fail.
>>
>> Meanwhile, the patch will trigger an existing memory write overflow, which will
>> casue segfault, please see the next patch.
>>
>> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  misc/tune2fs.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 065b483..91dc7c1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2908,8 +2908,7 @@ retry_open:
>>  				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>>  			rewrite_checksums = 1;
>>  	}
>> -	if (rewrite_checksums)
>> -		rewrite_metadata_checksums(fs);
>> +
>>  	if (I_flag) {
>>  		if (mount_flags & EXT2_MF_MOUNTED) {
>>  			fputs(_("The inode size may only be "
>> @@ -2935,6 +2934,7 @@ retry_open:
>>  		if (resize_inode(fs, new_inode_size) == 0) {
>>  			printf(_("Setting inode size %lu\n"),
>>  							new_inode_size);
>> +			rewrite_checksums = 1;
>>  		} else {
>>  			printf("%s", _("Failed to change inode size\n"));
>>  			rc = 1;
>> @@ -2942,6 +2942,9 @@ retry_open:
>>  		}
>>  	}
>>  
>> +	if (rewrite_checksums)
>> +		rewrite_metadata_checksums(fs);
>> +
> 
> Aha!  expand_inode_table() fails to recompute the checksums of the inode blocks
> it's moving around, and happily your change takes care of recomputing the inode
> checksums for a metadata_csum FS.  The changelog for this patch doesn't mention
> this, but it should.
Oh, I had not realized the inode checksum, thanks for pointing this.
So we agree to call a rewrite_metadata_checksums() here, in new version patch, i'll
update the changelog :)

> 
> There ought to be a regression test for this.  Can you send one along, please?
Sure, I spent some time about how to write a test, will send this test soon.

Regards,
Xiaoguang Wang
> 
> I crafted my own test case for the metadata_csum failure while trying to figure
> out what this patchset does, so I'll simply send it out.  Unfortunately, the
> test requires a fix for a bug in the hugefile code that I'll cram onto the
> patchbomb.
> 
> Other than that, you can add for all three patches:
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> --D
> 
>>  	if (l_flag)
>>  		list_super(sb);
>>  	if (stride_set) {
>> -- 
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

[PATCH v2 3/3] e2fsprogs/tune2fs: fix memory write overflow

[Xiaoguang Wang]

If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
we will trigger a segfault, this is because of the inode cache issues.

Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
to new inode size(for example, 256).

Then we re-compute metadata checksums, see below code flow:
|-->rewrite_metadata_checksums
|----->rewrite_inodes
|-------->ext2fs_write_inode_full
In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
	/* Check to see if the inode cache needs to be updated */
	if (fs->icache) {
		for (i=0; i < fs->icache->cache_size; i++) {
			if (fs->icache->cache[i].ino == ino) {
				memcpy(fs->icache->cache[i].inode, inode,
				       (bufsize > length) ? length : bufsize);
				break;
			}
		}
	}

Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
old inode size(for example, 128), but here the memcpy will obviously write overflow,
'(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
wrong, we need to fix this.
I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
inode cache, because inode size has changed, if necessary, we will re-create this inode cache.

Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
	dd if=/dev/zero of=file.img bs=1M count=128
	device_name=$(/sbin/losetup -f)
	/sbin/losetup -f file.img
	mkfs.ext4 -I 128 -O ^flex_bg $device_name
	tune2fs -I 256 $device_name

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
---
 misc/tune2fs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 91dc7c1..34d2cd9 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)
 
 	/* Update the meta data */
 	fs->inode_blocks_per_group = new_ino_blks_per_grp;
+	ext2fs_free_inode_cache(fs->icache);
+	fs->icache = 0;
 	fs->super->s_inode_size = new_ino_size;
 
 err_out:
-- 
1.8.2.1

[PATCH v3 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

[Xiaoguang Wang]

When we use ext2fs_open_inode_scan() to iterate inodes and finish
jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
inode_scan_and_fix(), we forgot to call it, fix this error.

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 7fee870..065b483 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2147,6 +2147,7 @@ static int inode_scan_and_fix(ext2_filsys fs, ext2fs_block_bitmap bmap)
 
 err_out:
 	ext2fs_free_mem(&block_buf);
+	ext2fs_close_inode_scan(scan);
 
 	return retval;
 }
-- 
1.8.2.1

[PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Xiaoguang Wang]

When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
obviously, we need to re-compute the group descriptor checksums, and the inode
's size has also changed, we also need to recompute the checksums of inodes for
metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
this will fix checksum issues.

Meanwhile, the patch will trigger an existing memory write overflow, which will
casue segfault, please see the next patch.

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 065b483..91dc7c1 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2908,8 +2908,7 @@ retry_open:
 				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
 			rewrite_checksums = 1;
 	}
-	if (rewrite_checksums)
-		rewrite_metadata_checksums(fs);
+
 	if (I_flag) {
 		if (mount_flags & EXT2_MF_MOUNTED) {
 			fputs(_("The inode size may only be "
@@ -2935,6 +2934,7 @@ retry_open:
 		if (resize_inode(fs, new_inode_size) == 0) {
 			printf(_("Setting inode size %lu\n"),
 							new_inode_size);
+			rewrite_checksums = 1;
 		} else {
 			printf("%s", _("Failed to change inode size\n"));
 			rc = 1;
@@ -2942,6 +2942,9 @@ retry_open:
 		}
 	}
 
+	if (rewrite_checksums)
+		rewrite_metadata_checksums(fs);
+
 	if (l_flag)
 		list_super(sb);
 	if (stride_set) {
-- 
1.8.2.1

Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Darrick J. Wong]

On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, and the inode
> 's size has also changed, we also need to recompute the checksums of inodes for
> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
> this will fix checksum issues.
> 
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  misc/tune2fs.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 065b483..91dc7c1 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2908,8 +2908,7 @@ retry_open:
>  				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>  			rewrite_checksums = 1;
>  	}
> -	if (rewrite_checksums)
> -		rewrite_metadata_checksums(fs);
> +
>  	if (I_flag) {
>  		if (mount_flags & EXT2_MF_MOUNTED) {
>  			fputs(_("The inode size may only be "
> @@ -2935,6 +2934,7 @@ retry_open:
>  		if (resize_inode(fs, new_inode_size) == 0) {

Come to think of it, if we're enabling metadata_csum /and/ resizing the inode,
we have to set EXT2_FLAG_IGNORE_CSUM_ERRORS before calling resize_inode.  If we
don't, the library calls that resize_inode() makes will try to verify the
checksums (which haven't been set yet) and the operation will fail.

Will send patch, but at this point I'm wondering if it doesn't make more sense
for me to incorporate them into my patchbomb rather than let this series get
lost in the blizzard...

--D

>  			printf(_("Setting inode size %lu\n"),
>  							new_inode_size);
> +			rewrite_checksums = 1;
>  		} else {
>  			printf("%s", _("Failed to change inode size\n"));
>  			rc = 1;
> @@ -2942,6 +2942,9 @@ retry_open:
>  		}
>  	}
>  
> +	if (rewrite_checksums)
> +		rewrite_metadata_checksums(fs);
> +
>  	if (l_flag)
>  		list_super(sb);
>  	if (stride_set) {
> -- 
> 1.8.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Xiaoguang Wang]

[-- Attachment #1: Type: text/plain, Size: 2753 bytes --]

Hi,

On 11/15/2014 07:10 AM, Darrick J. Wong wrote:
> On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
>> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
>> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
>> obviously, we need to re-compute the group descriptor checksums, and the inode
>> 's size has also changed, we also need to recompute the checksums of inodes for
>> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
>> this will fix checksum issues.
>>
>> Meanwhile, the patch will trigger an existing memory write overflow, which will
>> casue segfault, please see the next patch.
>>
>> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
>> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>> ---
>>  misc/tune2fs.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
>> index 065b483..91dc7c1 100644
>> --- a/misc/tune2fs.c
>> +++ b/misc/tune2fs.c
>> @@ -2908,8 +2908,7 @@ retry_open:
>>  				EXT4_FEATURE_RO_COMPAT_METADATA_CSUM))
>>  			rewrite_checksums = 1;
>>  	}
>> -	if (rewrite_checksums)
>> -		rewrite_metadata_checksums(fs);
>> +
>>  	if (I_flag) {
>>  		if (mount_flags & EXT2_MF_MOUNTED) {
>>  			fputs(_("The inode size may only be "
>> @@ -2935,6 +2934,7 @@ retry_open:
>>  		if (resize_inode(fs, new_inode_size) == 0) {
> 
> Come to think of it, if we're enabling metadata_csum /and/ resizing the inode,
> we have to set EXT2_FLAG_IGNORE_CSUM_ERRORS before calling resize_inode.  If we
> don't, the library calls that resize_inode() makes will try to verify the
> checksums (which haven't been set yet) and the operation will fail.

Aha, right, sorry!
> 
> Will send patch, but at this point I'm wondering if it doesn't make more sense
> for me to incorporate them into my patchbomb rather than let this series get
> lost in the blizzard...

OK, it would be better for you to incorporate them into your patchbomb, thanks!
Also I attached one filefrag's bug fix, please incorporate it too :)

Regards,
Xiaoguang Wang
> 
> --D
> 
>>  			printf(_("Setting inode size %lu\n"),
>>  							new_inode_size);
>> +			rewrite_checksums = 1;
>>  		} else {
>>  			printf("%s", _("Failed to change inode size\n"));
>>  			rc = 1;
>> @@ -2942,6 +2942,9 @@ retry_open:
>>  		}
>>  	}
>>  
>> +	if (rewrite_checksums)
>> +		rewrite_metadata_checksums(fs);
>> +
>>  	if (l_flag)
>>  		list_super(sb);
>>  	if (stride_set) {
>> -- 
>> 1.8.2.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 


[-- Attachment #2: 0001-filefrag-fix-wrong-extent-count-calculation-when-usi.patch --]
[-- Type: text/x-patch, Size: 4246 bytes --]

>From caf9da020d1bb67fac20cee2a020dd3cc33777ec Mon Sep 17 00:00:00 2001
From: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Date: Tue, 7 Oct 2014 16:31:17 +0800
Subject: [PATCH] filefrag: fix wrong extent count calculation when using
 FIBMAP

When using FIBMAP and '-e' option is specified, the calculation for fiemap_extent
is wrong, we wrongly updated fm_ext.fe_logical for every iteration, please see the
code in the end of 'for' loop in fm_ext.fe_logical().

In an ext2 file system(block size is 1024 bytes),
  dd if=/dev/zero of=testfile bs=1k count=15
  Using debugfs, corresponding physical blocks are "2049 2050 2051 2052 2053 2054
2055 2056 2057 2058 2059 2060 1025 2061 2062 2063",  1025 is this indirect block.
Before this patch, filefrag's output would be:
  filefrag -B -e testfile
  Filesystem type is: ef53
  Filesystem cylinder groups approximately 16
  File size of testfile is 15360 (15 blocks of 1024 bytes)
   ext:     logical_offset:        physical_offset: length:   expected: flags:
     0:        1..       2:       2050..      2051:      2:       2051: merged
     1:        3..       4:       2052..      2053:      2:       2053: merged
     2:        5..       6:       2054..      2055:      2:       2055: merged
     3:        7..       8:       2056..      2057:      2:       2057: merged
     4:        9..      10:       2058..      2059:      2:       2059: merged
     5:       11..      12:       2060..      2061:      2:       2062: merged
     6:       13..      14:       2062..      2063:      2:       2063: merged,eof
     7:       14..      14:       2063..      2063:      1:       2063: merged,eof
This output is not reasonable.

Fix this bug and try to make it readable. With this patch, the output would be:
  ./filefrag -B -e mntpoint/testfile
  Filesystem type is: ef53
  Filesystem cylinder groups approximately 16
  File size of mntpoint/testfile is 15360 (15 blocks of 1024 bytes)
   ext:     logical_offset:        physical_offset: length:   expected: flags:
     0:        0..      11:       2049..      2060:     12:       2062: merged
     1:       12..      14:       2061..      2063:      3:       2063: merged,eof
  mntpoint/testfile: 2 extents found, perfection would be 1 extent

Changes since v1->v2
 - remove the unnecessary {} for if statement.

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/filefrag.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/misc/filefrag.c b/misc/filefrag.c
index c1a8684..76e613f 100644
--- a/misc/filefrag.c
+++ b/misc/filefrag.c
@@ -315,32 +315,40 @@ static int filefrag_fibmap(int fd, int blk_shift, int *num_extents,
 			return rc;
 		if (block == 0)
 			continue;
+		count++;
+
 		if (*num_extents == 0) {
 			(*num_extents)++;
 			if (force_extent) {
 				print_extent_header();
+				fm_ext.fe_logical = logical;
 				fm_ext.fe_physical = block * st->st_blksize;
+				fm_ext.fe_length = st->st_blksize;
 			}
+			last_block = block;
+			continue;
 		}
-		count++;
-		if (force_extent && last_block != 0 &&
-		    (block != last_block + 1 ||
-		     fm_ext.fe_logical + fm_ext.fe_length != logical)) {
-			print_extent_info(&fm_ext, *num_extents - 1,
-					  (last_block + 1) * st->st_blksize,
-					  blk_shift, st);
-			fm_ext.fe_length = 0;
-			(*num_extents)++;
+
+		if (force_extent) {
+			if (block != last_block + 1 ||
+			    fm_ext.fe_length + fm_ext.fe_logical != logical) {
+				print_extent_info(&fm_ext, *num_extents - 1,
+						  (last_block + 1) *
+						  st->st_blksize,
+						  blk_shift, st);
+				fm_ext.fe_logical = logical;
+				fm_ext.fe_physical = block * st->st_blksize;
+				fm_ext.fe_length = st->st_blksize;
+				(*num_extents)++;
+			} else {
+				fm_ext.fe_length += st->st_blksize;
+			}
 		} else if (last_block && (block != last_block + 1)) {
 			if (verbose)
 				printf("Discontinuity: Block %ld is at %lu (was "
 				       "%lu)\n", i, block, last_block + 1);
-			fm_ext.fe_length = 0;
 			(*num_extents)++;
 		}
-		fm_ext.fe_logical = logical;
-		fm_ext.fe_physical = block * st->st_blksize;
-		fm_ext.fe_length += st->st_blksize;
 		last_block = block;
 	}
 
-- 
1.8.3.1

[PATCH] tune2fs: disable csum verification before resizing inode

[Darrick J. Wong]

When we're turning on metadata checksumming /and/ resizing the inode
at the same time, disable checksum verification during the
resize_inode() call because the subroutines it calls will try to
verify the checksums (which have not yet been set), causing the
operation to fail unnecessarily.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c               |    7 +++-
 tests/t_iexpand_mcsum/expect |   57 ++++++++++++++++++++++++++++++
 tests/t_iexpand_mcsum/name   |    1 +
 tests/t_iexpand_mcsum/script |   80 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 tests/t_iexpand_mcsum/expect
 create mode 100644 tests/t_iexpand_mcsum/name
 create mode 100644 tests/t_iexpand_mcsum/script

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index b510c49..f01b05b 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2961,8 +2961,13 @@ retry_open:
 		 * We want to update group descriptor also
 		 * with the new free inode count
 		 */
+		if (rewrite_checksums)
+			fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
 		fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
-		if (resize_inode(fs, new_inode_size) == 0) {
+		retval = resize_inode(fs, new_inode_size);
+		if (rewrite_checksums)
+			fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
+		if (retval == 0) {
 			printf(_("Setting inode size %lu\n"),
 							new_inode_size);
 			rewrite_checksums = 1;
diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
new file mode 100644
index 0000000..2bebc9f
--- /dev/null
+++ b/tests/t_iexpand_mcsum/expect
@@ -0,0 +1,57 @@
+tune2fs test
+Creating filesystem with 786432 1k blocks and 98304 inodes
+Superblock backups stored on blocks: 
+	8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
+
+Allocating group tables:      \b\b\b\b\bdone                            
+Writing inode tables:      \b\b\b\b\bdone                            
+Creating journal (16384 blocks): done
+Creating 6334 huge file(s) with 117 blocks each: done
+Writing superblocks and filesystem accounting information:      \b\b\b\b\bdone
+
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+Exit status is 0
+tune2fs -I 256 -O metadata_csum test.img
+Setting inode size 256
+
+Please run e2fsck -D on the filesystem.
+
+Exit status is 0
+Backing up journal inode block information.
+
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 3A: Optimizing directories
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+
+
+Change in FS metadata:
+@@ -5 +5 @@
+-Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
++Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
+@@ -13 +13 @@
+-Free blocks:              16280
++Free blocks:              3958
+@@ -22 +22 @@
+-Inode blocks per group:   128
++Inode blocks per group:   256
+@@ -28 +28 @@
+-Inode size:	          128
++Inode size:	          256
+@@ -31,0 +32 @@
++Checksum type:            crc32c
+Pass 1: Checking inodes, blocks, and sizes
+Pass 2: Checking directory structure
+Pass 3: Checking directory connectivity
+Pass 4: Checking reference counts
+Pass 5: Checking group summary information
+
+Exit status is 0
diff --git a/tests/t_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
new file mode 100644
index 0000000..e767715
--- /dev/null
+++ b/tests/t_iexpand_mcsum/name
@@ -0,0 +1 @@
+expand inodes and turn on metadata_csum
diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
new file mode 100644
index 0000000..cb424ed
--- /dev/null
+++ b/tests/t_iexpand_mcsum/script
@@ -0,0 +1,80 @@
+if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+
+FSCK_OPT=-fn
+OUT=$test_name.log
+EXP=$test_dir/expect
+CONF=$TMPFILE.conf
+
+#gzip -d < $EXP.gz > $EXP
+
+cat > $CONF << ENDL
+[fs_types]
+	ext4h = {
+		features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
+		blocksize = 1024
+		inode_size = 256
+		make_hugefiles = true
+		hugefiles_dir = /
+		hugefiles_slack = 16000K
+		hugefiles_name = aaaaa
+		hugefiles_digits = 4
+		hugefiles_size = 117K
+		zero_hugefiles = false
+	}
+ENDL
+
+echo "tune2fs test" > $OUT
+
+MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
+rm -rf $CONF
+
+# dump and check
+($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+# convert it
+echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
+dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
+$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
+
+# dump and check
+($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
+echo "Change in FS metadata:" >> $OUT
+diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
+$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
+status=$?
+echo Exit status is $status >> $OUT
+
+rm $TMPFILE
+
+#
+# Do the verification
+#
+
+sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
+mv $OUT.new $OUT
+
+cmp -s $OUT $EXP
+status=$?
+
+if [ "$status" = 0 ] ; then
+	echo "$test_name: $test_description: ok"
+	touch $test_name.ok
+else
+	echo "$test_name: $test_description: failed"
+	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
+fi
+
+rm $OUT.before $OUT.after
+
+unset IMAGE FSCK_OPT OUT EXP CONF
+
+else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
+	echo "$test_name: $test_description: skipped"
+fi
+

Re: [PATCH] tune2fs: disable csum verification before resizing inode

[Theodore Ts'o]

On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> When we're turning on metadata checksumming /and/ resizing the inode
> at the same time, disable checksum verification during the
> resize_inode() call because the subroutines it calls will try to
> verify the checksums (which have not yet been set), causing the
> operation to fail unnecessarily.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

						- Ted

Re: [PATCH] tune2fs: disable csum verification before resizing inode

[Theodore Ts'o]

On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> When we're turning on metadata checksumming /and/ resizing the inode
> at the same time, disable checksum verification during the
> resize_inode() call because the subroutines it calls will try to
> verify the checksums (which have not yet been set), causing the
> operation to fail unnecessarily.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

The t_iexpand_mcsum test is failing when I try applying this patch:

--- /usr/projects/e2fsprogs/e2fsprogs/tests/t_iexpand_mcsum/expect	2014-12-03 03:33:08.359025380 +0000
+++ t_iexpand_mcsum.log	2014-12-03 03:38:42.519031358 +0000
@@ -26,9 +26,388 @@
 
 Pass 1: Checking inodes, blocks, and sizes
 Pass 2: Checking directory structure
+Directory inode 2, block #0, offset 1004: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #116, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #117, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #118, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #119, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #120, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #121, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #122, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #123, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #1, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #2, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #3, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #4, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #5, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #6, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #7, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #8, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #9, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #10, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #11, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #12, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #13, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #14, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #15, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #16, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #17, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #18, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #19, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #20, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #21, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #22, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #23, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #24, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #25, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #26, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #27, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #28, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #29, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #30, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #31, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #32, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #33, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #34, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #35, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #36, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #37, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #38, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #39, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #40, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #41, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #42, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #43, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #44, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #45, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #46, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #47, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #48, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #49, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #50, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #51, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #52, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #53, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #54, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #55, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #56, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #57, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #58, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #59, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #60, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #61, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #62, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #63, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #64, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #65, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #66, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #67, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #68, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #69, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #70, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #71, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #72, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #73, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #74, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #75, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #76, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #77, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #78, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #79, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #80, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #81, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #82, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #83, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #84, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #85, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #86, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #87, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #88, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #89, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #90, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #91, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #92, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #93, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #94, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #95, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #96, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #97, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #98, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #99, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #100, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #101, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #102, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #103, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #104, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #105, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #106, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #107, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #108, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #109, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #110, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #111, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #112, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #113, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #114, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
+Directory inode 2, block #115, offset 1000: directory passes checks but fails checksum.
+Fix? yes
+
 Pass 3: Checking directory connectivity
+Error while trying to find /lost+found: Directory block checksum does not match directory block
+/lost+found not found.  Create? yes
+
+Error creating /lost+found directory (ext2fs_link): Directory block checksum does not match directory block
 Pass 3A: Optimizing directories
 Pass 4: Checking reference counts
+Unattached inode 6346
+Connect to /lost+found? yes
+
 Pass 5: Checking group summary information
 
 
@@ -37,9 +416,11 @@
 @@ -5 +5 @@
 -Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
 +Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
-@@ -13 +13 @@
--Free blocks:              16280
-+Free blocks:              3958
+@@ -13,2 +13,2 @@
+-Free blocks:              16282
+-Free inodes:              91959
++Free blocks:              3970
++Free inodes:              91958
 @@ -22 +22 @@
 -Inode blocks per group:   128
 +Inode blocks per group:   256
@@ -51,7 +432,17 @@
 Pass 1: Checking inodes, blocks, and sizes
 Pass 2: Checking directory structure
 Pass 3: Checking directory connectivity
+'..' in /lost+found/#6346 (6346) is / (2), should be /lost+found (11).
+Fix? no
+
 Pass 4: Checking reference counts
+Inode 2 ref count is 3, should be 4.  Fix? no
+
+Inode 6346 ref count is 3, should be 2.  Fix? no
+
 Pass 5: Checking group summary information
 
-Exit status is 0
+
+
+
+Exit status is 4


> ---
>  misc/tune2fs.c               |    7 +++-
>  tests/t_iexpand_mcsum/expect |   57 ++++++++++++++++++++++++++++++
>  tests/t_iexpand_mcsum/name   |    1 +
>  tests/t_iexpand_mcsum/script |   80 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 144 insertions(+), 1 deletion(-)
>  create mode 100644 tests/t_iexpand_mcsum/expect
>  create mode 100644 tests/t_iexpand_mcsum/name
>  create mode 100644 tests/t_iexpand_mcsum/script
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index b510c49..f01b05b 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2961,8 +2961,13 @@ retry_open:
>  		 * We want to update group descriptor also
>  		 * with the new free inode count
>  		 */
> +		if (rewrite_checksums)
> +			fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
>  		fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
> -		if (resize_inode(fs, new_inode_size) == 0) {
> +		retval = resize_inode(fs, new_inode_size);
> +		if (rewrite_checksums)
> +			fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
> +		if (retval == 0) {
>  			printf(_("Setting inode size %lu\n"),
>  							new_inode_size);
>  			rewrite_checksums = 1;
> diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
> new file mode 100644
> index 0000000..2bebc9f
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/expect
> @@ -0,0 +1,57 @@
> +tune2fs test
> +Creating filesystem with 786432 1k blocks and 98304 inodes
> +Superblock backups stored on blocks: 
> +	8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
> +
> +Allocating group tables:      \b\b\b\b\bdone                            
> +Writing inode tables:      \b\b\b\b\bdone                            
> +Creating journal (16384 blocks): done
> +Creating 6334 huge file(s) with 117 blocks each: done
> +Writing superblocks and filesystem accounting information:      \b\b\b\b\bdone
> +
> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +
> +Exit status is 0
> +tune2fs -I 256 -O metadata_csum test.img
> +Setting inode size 256
> +
> +Please run e2fsck -D on the filesystem.
> +
> +Exit status is 0
> +Backing up journal inode block information.
> +
> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 3A: Optimizing directories
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +
> +
> +
> +Change in FS metadata:
> +@@ -5 +5 @@
> +-Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> ++Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> +@@ -13 +13 @@
> +-Free blocks:              16280
> ++Free blocks:              3958
> +@@ -22 +22 @@
> +-Inode blocks per group:   128
> ++Inode blocks per group:   256
> +@@ -28 +28 @@
> +-Inode size:	          128
> ++Inode size:	          256
> +@@ -31,0 +32 @@
> ++Checksum type:            crc32c
> +Pass 1: Checking inodes, blocks, and sizes
> +Pass 2: Checking directory structure
> +Pass 3: Checking directory connectivity
> +Pass 4: Checking reference counts
> +Pass 5: Checking group summary information
> +
> +Exit status is 0
> diff --git a/tests/t_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
> new file mode 100644
> index 0000000..e767715
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/name
> @@ -0,0 +1 @@
> +expand inodes and turn on metadata_csum
> diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
> new file mode 100644
> index 0000000..cb424ed
> --- /dev/null
> +++ b/tests/t_iexpand_mcsum/script
> @@ -0,0 +1,80 @@
> +if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> +
> +FSCK_OPT=-fn
> +OUT=$test_name.log
> +EXP=$test_dir/expect
> +CONF=$TMPFILE.conf
> +
> +#gzip -d < $EXP.gz > $EXP
> +
> +cat > $CONF << ENDL
> +[fs_types]
> +	ext4h = {
> +		features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
> +		blocksize = 1024
> +		inode_size = 256
> +		make_hugefiles = true
> +		hugefiles_dir = /
> +		hugefiles_slack = 16000K
> +		hugefiles_name = aaaaa
> +		hugefiles_digits = 4
> +		hugefiles_size = 117K
> +		zero_hugefiles = false
> +	}
> +ENDL
> +
> +echo "tune2fs test" > $OUT
> +
> +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
> +rm -rf $CONF
> +
> +# dump and check
> +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +# convert it
> +echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
> +dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
> +$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
> +
> +# dump and check
> +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
> +echo "Change in FS metadata:" >> $OUT
> +diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
> +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> +status=$?
> +echo Exit status is $status >> $OUT
> +
> +rm $TMPFILE
> +
> +#
> +# Do the verification
> +#
> +
> +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> +mv $OUT.new $OUT
> +
> +cmp -s $OUT $EXP
> +status=$?
> +
> +if [ "$status" = 0 ] ; then
> +	echo "$test_name: $test_description: ok"
> +	touch $test_name.ok
> +else
> +	echo "$test_name: $test_description: failed"
> +	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
> +fi
> +
> +rm $OUT.before $OUT.after
> +
> +unset IMAGE FSCK_OPT OUT EXP CONF
> +
> +else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> +	echo "$test_name: $test_description: skipped"
> +fi
> +

Re: [PATCH] tune2fs: disable csum verification before resizing inode

[Darrick J. Wong]

On Tue, Dec 02, 2014 at 10:46:49PM -0500, Theodore Ts'o wrote:
> On Mon, Nov 17, 2014 at 11:10:39AM -0800, Darrick J. Wong wrote:
> > When we're turning on metadata checksumming /and/ resizing the inode
> > at the same time, disable checksum verification during the
> > resize_inode() call because the subroutines it calls will try to
> > verify the checksums (which have not yet been set), causing the
> > operation to fail unnecessarily.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> The t_iexpand_mcsum test is failing when I try applying this patch:
> 
> --- /usr/projects/e2fsprogs/e2fsprogs/tests/t_iexpand_mcsum/expect	2014-12-03 03:33:08.359025380 +0000
> +++ t_iexpand_mcsum.log	2014-12-03 03:38:42.519031358 +0000
> @@ -26,9 +26,388 @@
>  
>  Pass 1: Checking inodes, blocks, and sizes
>  Pass 2: Checking directory structure
> +Directory inode 2, block #0, offset 1004: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #116, offset 1000: directory passes checks but fails checksum.
> +Fix? yes

Arrrgh.  The fun of maintaining the test suite when the patches go into -next
in a different order from where they are in my tree... :)

The patches "e2fsck: only complain about no-checksum directory blocks once" and
"e2fsck: don't complain about root dir csum failures when getting lnf" fix fsck
not to print these unnecessary error messages.  They both pre-date the creation
of this patch, which is why the test is failing.  You can safely amend the
/expect file, but it'll be necessary to un-amend it if you take those two older
patches.

:)

--D

> +
> +Directory inode 2, block #117, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #118, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #119, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #120, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #121, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #122, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #123, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #1, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #2, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #3, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #4, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #5, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #6, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #7, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #8, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #9, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #10, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #11, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #12, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #13, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #14, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #15, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #16, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #17, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #18, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #19, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #20, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #21, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #22, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #23, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #24, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #25, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #26, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #27, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #28, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #29, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #30, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #31, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #32, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #33, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #34, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #35, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #36, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #37, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #38, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #39, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #40, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #41, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #42, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #43, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #44, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #45, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #46, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #47, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #48, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #49, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #50, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #51, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #52, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #53, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #54, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #55, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #56, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #57, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #58, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #59, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #60, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #61, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #62, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #63, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #64, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #65, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #66, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #67, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #68, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #69, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #70, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #71, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #72, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #73, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #74, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #75, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #76, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #77, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #78, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #79, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #80, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #81, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #82, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #83, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #84, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #85, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #86, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #87, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #88, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #89, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #90, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #91, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #92, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #93, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #94, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #95, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #96, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #97, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #98, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #99, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #100, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #101, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #102, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #103, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #104, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #105, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #106, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #107, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #108, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #109, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #110, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #111, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #112, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #113, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #114, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
> +Directory inode 2, block #115, offset 1000: directory passes checks but fails checksum.
> +Fix? yes
> +
>  Pass 3: Checking directory connectivity
> +Error while trying to find /lost+found: Directory block checksum does not match directory block
> +/lost+found not found.  Create? yes
> +
> +Error creating /lost+found directory (ext2fs_link): Directory block checksum does not match directory block
>  Pass 3A: Optimizing directories
>  Pass 4: Checking reference counts
> +Unattached inode 6346
> +Connect to /lost+found? yes
> +
>  Pass 5: Checking group summary information
>  
>  
> @@ -37,9 +416,11 @@
>  @@ -5 +5 @@
>  -Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
>  +Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> -@@ -13 +13 @@
> --Free blocks:              16280
> -+Free blocks:              3958
> +@@ -13,2 +13,2 @@
> +-Free blocks:              16282
> +-Free inodes:              91959
> ++Free blocks:              3970
> ++Free inodes:              91958
>  @@ -22 +22 @@
>  -Inode blocks per group:   128
>  +Inode blocks per group:   256
> @@ -51,7 +432,17 @@
>  Pass 1: Checking inodes, blocks, and sizes
>  Pass 2: Checking directory structure
>  Pass 3: Checking directory connectivity
> +'..' in /lost+found/#6346 (6346) is / (2), should be /lost+found (11).
> +Fix? no
> +
>  Pass 4: Checking reference counts
> +Inode 2 ref count is 3, should be 4.  Fix? no
> +
> +Inode 6346 ref count is 3, should be 2.  Fix? no
> +
>  Pass 5: Checking group summary information
>  
> -Exit status is 0
> +
> +
> +
> +Exit status is 4
> 
> 
> > ---
> >  misc/tune2fs.c               |    7 +++-
> >  tests/t_iexpand_mcsum/expect |   57 ++++++++++++++++++++++++++++++
> >  tests/t_iexpand_mcsum/name   |    1 +
> >  tests/t_iexpand_mcsum/script |   80 ++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 144 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/t_iexpand_mcsum/expect
> >  create mode 100644 tests/t_iexpand_mcsum/name
> >  create mode 100644 tests/t_iexpand_mcsum/script
> > 
> > diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> > index b510c49..f01b05b 100644
> > --- a/misc/tune2fs.c
> > +++ b/misc/tune2fs.c
> > @@ -2961,8 +2961,13 @@ retry_open:
> >  		 * We want to update group descriptor also
> >  		 * with the new free inode count
> >  		 */
> > +		if (rewrite_checksums)
> > +			fs->flags |= EXT2_FLAG_IGNORE_CSUM_ERRORS;
> >  		fs->flags &= ~EXT2_FLAG_SUPER_ONLY;
> > -		if (resize_inode(fs, new_inode_size) == 0) {
> > +		retval = resize_inode(fs, new_inode_size);
> > +		if (rewrite_checksums)
> > +			fs->flags &= ~EXT2_FLAG_IGNORE_CSUM_ERRORS;
> > +		if (retval == 0) {
> >  			printf(_("Setting inode size %lu\n"),
> >  							new_inode_size);
> >  			rewrite_checksums = 1;
> > diff --git a/tests/t_iexpand_mcsum/expect b/tests/t_iexpand_mcsum/expect
> > new file mode 100644
> > index 0000000..2bebc9f
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/expect
> > @@ -0,0 +1,57 @@
> > +tune2fs test
> > +Creating filesystem with 786432 1k blocks and 98304 inodes
> > +Superblock backups stored on blocks: 
> > +	8193, 24577, 40961, 57345, 73729, 204801, 221185, 401409, 663553
> > +
> > +Allocating group tables:      \b\b\b\b\bdone                            
> > +Writing inode tables:      \b\b\b\b\bdone                            
> > +Creating journal (16384 blocks): done
> > +Creating 6334 huge file(s) with 117 blocks each: done
> > +Writing superblocks and filesystem accounting information:      \b\b\b\b\bdone
> > +
> > +Pass 1: Checking inodes, blocks, and sizes
> > +Pass 2: Checking directory structure
> > +Pass 3: Checking directory connectivity
> > +Pass 4: Checking reference counts
> > +Pass 5: Checking group summary information
> > +
> > +Exit status is 0
> > +tune2fs -I 256 -O metadata_csum test.img
> > +Setting inode size 256
> > +
> > +Please run e2fsck -D on the filesystem.
> > +
> > +Exit status is 0
> > +Backing up journal inode block information.
> > +
> > +Pass 1: Checking inodes, blocks, and sizes
> > +Pass 2: Checking directory structure
> > +Pass 3: Checking directory connectivity
> > +Pass 3A: Optimizing directories
> > +Pass 4: Checking reference counts
> > +Pass 5: Checking group summary information
> > +
> > +
> > +
> > +Change in FS metadata:
> > +@@ -5 +5 @@
> > +-Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file uninit_bg dir_nlink extra_isize
> > ++Filesystem features:      has_journal ext_attr dir_index filetype extent 64bit sparse_super large_file huge_file dir_nlink extra_isize metadata_csum
> > +@@ -13 +13 @@
> > +-Free blocks:              16280
> > ++Free blocks:              3958
> > +@@ -22 +22 @@
> > +-Inode blocks per group:   128
> > ++Inode blocks per group:   256
> > +@@ -28 +28 @@
> > +-Inode size:	          128
> > ++Inode size:	          256
> > +@@ -31,0 +32 @@
> > ++Checksum type:            crc32c
> > +Pass 1: Checking inodes, blocks, and sizes
> > +Pass 2: Checking directory structure
> > +Pass 3: Checking directory connectivity
> > +Pass 4: Checking reference counts
> > +Pass 5: Checking group summary information
> > +
> > +Exit status is 0
> > diff --git a/tests/t_iexpand_mcsum/name b/tests/t_iexpand_mcsum/name
> > new file mode 100644
> > index 0000000..e767715
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/name
> > @@ -0,0 +1 @@
> > +expand inodes and turn on metadata_csum
> > diff --git a/tests/t_iexpand_mcsum/script b/tests/t_iexpand_mcsum/script
> > new file mode 100644
> > index 0000000..cb424ed
> > --- /dev/null
> > +++ b/tests/t_iexpand_mcsum/script
> > @@ -0,0 +1,80 @@
> > +if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> > +
> > +FSCK_OPT=-fn
> > +OUT=$test_name.log
> > +EXP=$test_dir/expect
> > +CONF=$TMPFILE.conf
> > +
> > +#gzip -d < $EXP.gz > $EXP
> > +
> > +cat > $CONF << ENDL
> > +[fs_types]
> > +	ext4h = {
> > +		features = has_journal,extent,huge_file,uninit_bg,dir_nlink,extra_isize,sparse_super,filetype,dir_index,ext_attr,^resize_inode,^meta_bg,^flex_bg,^metadata_csum,64bit
> > +		blocksize = 1024
> > +		inode_size = 256
> > +		make_hugefiles = true
> > +		hugefiles_dir = /
> > +		hugefiles_slack = 16000K
> > +		hugefiles_name = aaaaa
> > +		hugefiles_digits = 4
> > +		hugefiles_size = 117K
> > +		zero_hugefiles = false
> > +	}
> > +ENDL
> > +
> > +echo "tune2fs test" > $OUT
> > +
> > +MKE2FS_CONFIG=$CONF $MKE2FS -F -T ext4h -I 128 $TMPFILE 786432 >> $OUT 2>&1
> > +rm -rf $CONF
> > +
> > +# dump and check
> > +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.before 2> /dev/null
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +# convert it
> > +echo "tune2fs -I 256 -O metadata_csum test.img" >> $OUT
> > +dd if=/dev/zero of=$TMPFILE conv=notrunc bs=1 count=1 seek=3221225471 2> /dev/null
> > +$TUNE2FS -I 256 -O metadata_csum $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +$FSCK -N test_filesys -y -f -D $TMPFILE >> $OUT 2>&1
> > +
> > +# dump and check
> > +($DUMPE2FS -h $TMPFILE; $DUMPE2FS -g $TMPFILE) 2>&1 | sed -f $cmd_dir/filter.sed -e '/^Checksum:.*/d' >> $OUT.after 2> /dev/null
> > +echo "Change in FS metadata:" >> $OUT
> > +diff -u0 $OUT.before $OUT.after | sed -e '/^---.*/d' -e '/^+++.*/d' >> $OUT
> > +$FSCK $FSCK_OPT -N test_filesys $TMPFILE >> $OUT 2>&1
> > +status=$?
> > +echo Exit status is $status >> $OUT
> > +
> > +rm $TMPFILE
> > +
> > +#
> > +# Do the verification
> > +#
> > +
> > +sed -f $cmd_dir/filter.sed -e "s;$TMPFILE;test.img;" -e 's/test_filesys:.*//g' < $OUT > $OUT.new
> > +mv $OUT.new $OUT
> > +
> > +cmp -s $OUT $EXP
> > +status=$?
> > +
> > +if [ "$status" = 0 ] ; then
> > +	echo "$test_name: $test_description: ok"
> > +	touch $test_name.ok
> > +else
> > +	echo "$test_name: $test_description: failed"
> > +	diff $DIFF_OPTS $EXP $OUT > $test_name.failed
> > +fi
> > +
> > +rm $OUT.before $OUT.after
> > +
> > +unset IMAGE FSCK_OPT OUT EXP CONF
> > +
> > +else #if test -x $RESIZE2FS_EXE -a -x $DEBUGFS_EXE; then
> > +	echo "$test_name: $test_description: skipped"
> > +fi
> > +
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tune2fs: disable csum verification before resizing inode

[Theodore Ts'o]

On Tue, Dec 02, 2014 at 09:39:51PM -0800, Darrick J. Wong wrote:
> 
> The patches "e2fsck: only complain about no-checksum directory blocks once" and
> "e2fsck: don't complain about root dir csum failures when getting lnf" fix fsck
> not to print these unnecessary error messages.  They both pre-date the creation
> of this patch, which is why the test is failing.  You can safely amend the
> /expect file, but it'll be necessary to un-amend it if you take those two older
> patches.

Ah, I figured it was something like that.  OK, I'll find those patches
and get them into my tree; so I'll fix it on my end.

Thanks!!

    	     	     	      	       	  - Ted

Re: [PATCH v3 2/3] e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size

[Theodore Ts'o]

On Fri, Nov 14, 2014 at 05:15:36PM +0800, Xiaoguang Wang wrote:
> When we use tune2fs -I new_ino_size to change inode size, if everything is OK,
> the corresponding ext4_group_desc.bg_free_blocks_count will be decreased, so
> obviously, we need to re-compute the group descriptor checksums, and the inode
> 's size has also changed, we also need to recompute the checksums of inodes for
> metadata_csum filesystem, so here we choose to call a rewrite_metadata_checksums(),
> this will fix checksum issues.
> 
> Meanwhile, the patch will trigger an existing memory write overflow, which will
> casue segfault, please see the next patch.
> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

					- Ted

[PATCH v3 3/3] e2fsprogs/tune2fs: fix memory write overflow

[Xiaoguang Wang]

If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
we will trigger a segfault, this is because of the inode cache issues.

Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
to new inode size(for example, 256).

Then we re-compute metadata checksums, see below code flow:
|-->rewrite_metadata_checksums
|----->rewrite_inodes
|-------->ext2fs_write_inode_full
In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
	/* Check to see if the inode cache needs to be updated */
	if (fs->icache) {
		for (i=0; i < fs->icache->cache_size; i++) {
			if (fs->icache->cache[i].ino == ino) {
				memcpy(fs->icache->cache[i].inode, inode,
				       (bufsize > length) ? length : bufsize);
				break;
			}
		}
	}

Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
old inode size(for example, 128), but here the memcpy will obviously write overflow,
'(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
wrong, we need to fix this.
I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
inode cache, because inode size has changed, if necessary, we will re-create this inode cache.

Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
	dd if=/dev/zero of=file.img bs=1M count=128
	device_name=$(/sbin/losetup -f)
	/sbin/losetup -f file.img
	mkfs.ext4 -I 128 -O ^flex_bg $device_name
	tune2fs -I 256 $device_name

Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 misc/tune2fs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 91dc7c1..34d2cd9 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)
 
 	/* Update the meta data */
 	fs->inode_blocks_per_group = new_ino_blks_per_grp;
+	ext2fs_free_inode_cache(fs->icache);
+	fs->icache = 0;
 	fs->super->s_inode_size = new_ino_size;
 
 err_out:
-- 
1.8.2.1

Re: [PATCH v3 3/3] e2fsprogs/tune2fs: fix memory write overflow

[Darrick J. Wong]

On Fri, Nov 14, 2014 at 05:15:37PM +0800, Xiaoguang Wang wrote:
> If we apply this patch 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size',
> we will trigger a segfault, this is because of the inode cache issues.
> 
> Firstly we should notice that in expand_inode_table(), we have change the super block's s_inode_size
> to new inode size(for example, 256).
> 
> Then we re-compute metadata checksums, see below code flow:
> |-->rewrite_metadata_checksums
> |----->rewrite_inodes
> |-------->ext2fs_write_inode_full
> In ext2fs_write_inode_full(), if an inode cache is hit, the below code will be executed:
> 	/* Check to see if the inode cache needs to be updated */
> 	if (fs->icache) {
> 		for (i=0; i < fs->icache->cache_size; i++) {
> 			if (fs->icache->cache[i].ino == ino) {
> 				memcpy(fs->icache->cache[i].inode, inode,
> 				       (bufsize > length) ? length : bufsize);
> 				break;
> 			}
> 		}
> 	}
> 
> Before executing rewrite_inodes(), actually the inode in inode cache is allocated by
> old inode size(for example, 128), but here the memcpy will obviously write overflow,
> '(bufsize > length) ? length : bufsize' here will return 256(new inode size), so this is
> wrong, we need to fix this.
> I think we should call ext2fs_free_inode_cache() in expand_inode_table(), to drop the
> inode cache, because inode size has changed, if necessary, we will re-create this inode cache.
> 
> Steps to reproduce this bug(apply 'e2fsprogs/tune2fs: rewrite metadata checksums when resizing inode size' first).
> 	dd if=/dev/zero of=file.img bs=1M count=128
> 	device_name=$(/sbin/losetup -f)
> 	/sbin/losetup -f file.img
> 	mkfs.ext4 -I 128 -O ^flex_bg $device_name
> 	tune2fs -I 256 $device_name
> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  misc/tune2fs.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/misc/tune2fs.c b/misc/tune2fs.c
> index 91dc7c1..34d2cd9 100644
> --- a/misc/tune2fs.c
> +++ b/misc/tune2fs.c
> @@ -2244,6 +2244,8 @@ static int expand_inode_table(ext2_filsys fs, unsigned long new_ino_size)
>  
>  	/* Update the meta data */
>  	fs->inode_blocks_per_group = new_ino_blks_per_grp;
> +	ext2fs_free_inode_cache(fs->icache);
> +	fs->icache = 0;

By the way (and this is a general question, not one specific to just this
patch), why do some parts of e2fsprogs assign 0 (instead of NULL) to pointers?

--D

>  	fs->super->s_inode_size = new_ino_size;
>  
>  err_out:
> -- 
> 1.8.2.1
> 

Re: [PATCH v3 3/3] e2fsprogs/tune2fs: fix memory write overflow

[Theodore Ts'o]

Applied, thanks.

						- Ted

Re: [PATCH v3 1/3] e2fsprogs/tune2fs: fix memory leak in inode_scan_and_fix()

[Theodore Ts'o]

On Fri, Nov 14, 2014 at 05:15:35PM +0800, Xiaoguang Wang wrote:
> When we use ext2fs_open_inode_scan() to iterate inodes and finish
> jobs, we also need a ext2fs_close_inode_scan(scan) operation, but in
> inode_scan_and_fix(), we forgot to call it, fix this error.
> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

Applied, thanks.

					- Ted