New: journal_data mount option causes filesystem

Bug 9692 Jayson King 2 participants 2 messages Jan 7, 2008

lore

Share & tools

lore

Jayson King

author

2008-01-07 03:09 UTC #0 lore raw

Quoted 17 lines · Andrew Morton

Quoted 16 lines · On Sat, 5 Jan 2008 09:52:15 -0800 (PST) [email protected] wrote:

http://bugzilla.kernel.org/show_bug.cgi?id=9692

           Summary: journal_data mount option causes filesystem corruption
                    with blocksize != 4096
           Product: File System
           Version: 2.5
     KernelVersion: 2.6.23.9
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: ext3
        AssignedTo: [email protected]
        ReportedBy: [email protected]

This looks to be an off-by-one bug with e2fsck in the function check_blocks(), and there isn't any actual filesystem corruption (e2fsck causes the corruption).

Please see the attached patch, which fixes the problem for me.

Jayson King

Quoted 1 line

From 654f24814e7b80d3b16bec2a67c13c43cb20eb2f Mon Sep 17 00:00:00 2001

From: Jayson R. King <[email protected]>
Date: Sun, 6 Jan 2008 18:14:18 -0600
Subject: e2fsck: Fix off-by-one error in check_blocks()

e2fsck allows extra blocks to be allocated to an inode up to the next multiple of page size iff the block size is not equal to page size. An off-by-one error in checking for this causes e2fsck to wrongly detect a bad i_size for such inodes and results in incorrectly adjusting the i_size to include those blocks.

Signed-off-by Jayson R. King <[email protected]>

---
 e2fsck/pass1.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

e2fsck/pass1.c +1 -1

index 56218ae..7bf0686 100644
@@ -1593,7 +1593,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
 		if ((pb.last_block >= 0) &&
 		    /* allow allocated blocks to end of PAGE_SIZE */
 		    (size < (__u64)pb.last_block * fs->blocksize) &&
-		    (pb.last_block / blkpg * blkpg != pb.last_block ||
+		    ((pb.last_block+1) & (blkpg-1) != 0 ||
 		     size < (__u64)(pb.last_block & ~(blkpg-1)) *fs->blocksize))
 			bad_size = 3;
 		else if (size > ext2_max_sizes[fs->super->s_log_block_size])

Signature · 1 line

1.5.3.3

Andreas Dilger

2008-01-07 23:14 UTC #1 lore raw

Quoted 3 lines · Jayson King

This looks to be an off-by-one bug with e2fsck in the function check_blocks(), and there isn't any actual filesystem corruption (e2fsck causes the corruption).

This is actually a problem for cases where blocksize != pagesize. We have a similar patch in our e2fsprogs, and I thought we sent an equivalent patch to Ted previously...

- (pb.last_block / blkpg * blkpg != pb.last_block ||
+ ((pb.last_block+1) / blkpg * blkpg != (pb.last_block+1) ||

Signed-off-by Kalpak Shah <[email protected]>

Signed-off-by Andreas Dilger <[email protected]>

Quoted 33 lines

Quoted 1 line
From 654f24814e7b80d3b16bec2a67c13c43cb20eb2f Mon Sep 17 00:00:00 2001
From: Jayson R. King <[email protected]>
Date: Sun, 6 Jan 2008 18:14:18 -0600
Subject: e2fsck: Fix off-by-one error in check_blocks()
e2fsck allows extra blocks to be allocated to an inode up to the next multiple of page size iff the block size is not equal to page size. An off-by-one error in checking for this causes e2fsck to wrongly detect a bad i_size for such inodes and results in incorrectly adjusting the i_size to include those blocks.
Signed-off-by Jayson R. King <[email protected]>
---
 e2fsck/pass1.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
e2fsck/pass1.c +1 -1
index 56218ae..7bf0686 100644
@@ -1593,7 +1593,7 @@ static void check_blocks(e2fsck_t ctx, struct problem_context *pctx,
 		if ((pb.last_block >= 0) &&
 		    /* allow allocated blocks to end of PAGE_SIZE */
 		    (size < (__u64)pb.last_block * fs->blocksize) &&
-		    (pb.last_block / blkpg * blkpg != pb.last_block ||
+		    ((pb.last_block+1) & (blkpg-1) != 0 ||
 		     size < (__u64)(pb.last_block & ~(blkpg-1)) *fs->blocksize))
 			bad_size = 3;
 		else if (size > ext2_max_sizes[fs->super->s_log_block_size])
Signature · 1 line
1.5.3.3

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.