diff mbox

e2fsck: fix "can't find dup_blk" error

Message ID 1321584758-4259-1-git-send-email-hao.bigrat@gmail.com
State Superseded, archived
Headers show

Commit Message

Robin Dong Nov. 18, 2011, 2:52 a.m. UTC
From: Robin Dong <sanbai@taobao.com>

After:
1. mke2fs -O ^has_journal,^resize_inode,^uninit_bg,extent,meta_bg,flex_bg,bigalloc /dev/sda
2. mount -t ext4 /dev/sda /test/
3. create file (8192K size, extent' e_len is 2) /test/1 ~ /test/10
4. use tool to change the /test/5 extent's e_len to 100, corrupt file
5. e2fsck -f /dev/sda

And it will report like:

 Clone multiply-claimed blocks<y>? yes

 clone_file_block: internal error: can't find dup_blk for 525345

 clone_file_block: internal error: can't find dup_blk for 525346

 clone_file_block: internal error: can't find dup_blk for 525347

 clone_file_block: internal error: can't find dup_blk for 525348


In bigalloc filesystem, "blockcnt > 0" should change to "blockcnt >= cluster_ratio"

Signed-off-by: Robin Dong <sanbai@taobao.com>
---
 e2fsck/pass1b.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Theodore Ts'o Nov. 19, 2011, 4:02 a.m. UTC | #1
On Fri, Nov 18, 2011 at 10:52:38AM +0800, Robin Dong wrote:
> From: Robin Dong <sanbai@taobao.com>
> 
> After:
> 1. mke2fs -O ^has_journal,^resize_inode,^uninit_bg,extent,meta_bg,flex_bg,bigalloc /dev/sda
> 2. mount -t ext4 /dev/sda /test/
> 3. create file (8192K size, extent' e_len is 2) /test/1 ~ /test/10
> 4. use tool to change the /test/5 extent's e_len to 100, corrupt file
> 5. e2fsck -f /dev/sda

I can't replicate this.  I presume it only shows up with your
modification to the bigalloc format?

						- Ted
--
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
Robin Dong Nov. 19, 2011, 8:32 a.m. UTC | #2
2011/11/19 Ted Ts'o <tytso@mit.edu>:
> On Fri, Nov 18, 2011 at 10:52:38AM +0800, Robin Dong wrote:
>> From: Robin Dong <sanbai@taobao.com>
>>
>> After:
>> 1. mke2fs -O ^has_journal,^resize_inode,^uninit_bg,extent,meta_bg,flex_bg,bigalloc /dev/sda
>> 2. mount -t ext4 /dev/sda /test/
>> 3. create file (8192K size, extent' e_len is 2) /test/1 ~ /test/10
>> 4. use tool to change the /test/5 extent's e_len to 100, corrupt file
>> 5. e2fsck -f /dev/sda
>
> I can't replicate this.  I presume it only shows up with your
> modification to the bigalloc format?
>
>                                                - Ted
>

Hi, Ted

I am not using my modification bigalloc format.
I use the upstream kernel (3.2-rc2) and uptodate e2fsprogs master
branch code, the error report will emerge after my steps above.
My /dev/sda is small (about 100G), so I guess:

# create /test/1 ~ /test/100 (e_len == 2)
# make /test/5 's e_len to 1024

may replicate this problem on larger disk.
Theodore Ts'o Nov. 28, 2011, 5:02 p.m. UTC | #3
Ok, I was able to replicate it, and created a small test case (see
attached), but it turns out your patch wasn't sufficient.  It didn't
actually fix the problem correctly, and left the file system in a
corrupted state, which was seen when we ran e2fsck a second time and
the i_blocks field was not correctly.

Your patch also resulted in blocks within a logical cluster
incorrectly assigned to two physical clusters.  (I still need to add
some code in e2fsck to notice this particular corruption, but it's
highly unlikely for this to happen due to hardware errors --- just
kernel/e2fsck bugs.)

I'll attach to this mail thread the patches I used to fix this, plus
the test case.

     	       	    	 	    	      - Ted
diff mbox

Patch

diff --git a/e2fsck/pass1b.c b/e2fsck/pass1b.c
index d5585dd..a9cdd9e 100644
--- a/e2fsck/pass1b.c
+++ b/e2fsck/pass1b.c
@@ -703,7 +703,7 @@  static int clone_file_block(ext2_filsys fs,
 	if (check_if_fs_cluster(ctx, EXT2FS_B2C(fs, *block_nr)))
 		is_meta = 1;
 
-	if (((blockcnt > 0) && c == cs->dup_cluster) ||
+	if (((blockcnt >= EXT2FS_CLUSTER_RATIO(fs)) && c == cs->dup_cluster) ||
 	    ext2fs_test_block_bitmap2(ctx->block_dup_map, *block_nr)) {
 		n = dict_lookup(&clstr_dict,
 				INT_TO_VOIDPTR(EXT2FS_B2C(fs, *block_nr)));