Re: [Bug 9692] New: journal_data mount option causes filesystem

2008-01-07 Thread Andreas Dilger
On Jan 06, 2008  19:30 -0600, Jayson King wrote:
> 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]>

> >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(-)
> 
> diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
> index 56218ae..7bf0686 100644
> --- a/e2fsck/pass1.c
> +++ b/e2fsck/pass1.c
> @@ -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])
> -- 
> 1.5.3.3
> 
> 


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

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bug 9692] New: journal_data mount option causes filesystem

2008-01-06 Thread Jayson King

Andrew Morton wrote:
>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

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

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 56218ae..7bf0686 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -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])
-- 
1.5.3.3




Re: [Bug 9692] New: journal_data mount option causes filesystem corruption with blocksize != 4096

2008-01-05 Thread Andrew Morton
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]
> 
> 
> Most recent kernel where this bug did not occur: -
> Older kernels have this problem too (I think I noticed this booting >= 2.6.21,
> definitely 2.6.22).

I'm getting the feeling that we should just disable data=journal.  Make it
use data=ordered mode instead.  It isn't getting a lot of attention..

> Distribution: Gentoo Linux x86
> This bug seems to be hardware-independent (tested on three different machines
> which all use quite different drivers). If you need hardware information or 
> any
> other log or configuration files, let me know please.
> 
> Problem Description:
> When creating an ext3 filesystem with journal_data option and block sizes
> different than 4096 (tested: 1024, 2048) filesystem corruption will occur if
> certain operations are performed (see below).
> Corruption will not occur if 4096 block size is used, or if any other block
> size is used together with journal_data_ordered or journal_data_writeback.
> No errors in dmesg.
> 
> Steps to reproduce:
> I found this bug using an audio file tagger, so you need exfalso which is part
> of quodlibet (http://www.sacredchao.net/quodlibet/). No other file tagger I
> used produced this kind of problem. Still, this has to be a kernel problem,
> right??
> 
> 1. Create ext3 file system:
> mkfs.ext3 -O has_journal,dir_index -b 1024 /dev/sdd1
> tune2fs -c 0 -i 0 -m 0 -o journal_data /dev/sdd1
> 
> tune2fs 1.40.3 (05-Dec-2007)  (filtered)
> Filesystem volume name:   
> Last mounted on:  
> Filesystem magic number:  0xEF53
> Filesystem revision #:1 (dynamic)
> Filesystem features:  has_journal resize_inode dir_index filetype
> sparse_super
> Filesystem flags: signed directory hash
> Default mount options:journal_data
> Filesystem state: clean
> Errors behavior:  Continue
> Filesystem OS type:   Linux
> Inode count:  126976
> Block count:  1012060
> Reserved block count: 0
> Free blocks:  976865
> Free inodes:  126965
> First block:  1
> Block size:   1024
> Fragment size:1024
> Reserved GDT blocks:  256
> Blocks per group: 8192
> Fragments per group:  8192
> Inodes per group: 1024
> Inode blocks per group:   128
> Last mount time:  n/a
> Mount count:  0
> Maximum mount count:  -1
> Check interval:   0 ()
> Reserved blocks uid:  0 (user root)
> Reserved blocks gid:  0 (group root)
> First inode:  11
> Inode size:   128
> Journal inode:8
> Default directory hash:   tea
> Journal backup:   inode blocks
> 
> 2. Mount it and copy mp3,ogg,... files to it. This does not cause any file
> system corruption (which you can confirm by running fsck).
> 
> pmount /dev/sdd1:
> /dev/sdd1 on /media/sdd1 type ext3 (rw,noexec,nosuid,nodev,errors=continue)
> 
> 3. Use quodlibet/exfalso to change the id3 tags. Add tags to it if not 
> present,
> or delete them if already present. This will lead to file system corruption.
> 
> brw-r- 1 root disk 8, 49 /dev/sdd1
> 
> 4. Unmount the volume.
> pumount /dev/sdd1
> 
> 5. Run fsck -fvD /dev/sdd1. It will complain about wrong i_size.
> 
> e2fsck 1.40.3 (05-Dec-2007)
> Pass 1: Checking inodes, blocks, and sizes
> Inode 47106, i_size is 5015509, should be 5017600.  Fix? yes
> Inode 47107, i_size is 4657736, should be 4661248.  Fix? yes
> Inode 47109, i_size is 11928555, should be 11931648.  Fix? yes
> Inode 47111, i_size is 5698454, should be 5701632.  Fix? yes
> Inode 47112, i_size is 9384018, should be 9388032.  Fix? yes
> Inode 47114, i_size is 5679228, should be 5681152.  Fix? yes
> Inode 47115, i_size is 6107218, should be 6111232.  Fix? yes
> Inode 47117, i_size is 4354297, should be 4358144.  Fix? yes
> Inode 47118, i_size is 4512286, should be 4513792.  Fix? yes
> Inode 47120, i_size is 7010846, should be 7012352.  Fix? yes
> 
> 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
> 
> /dev/sdd1: * FILE SYSTEM WAS MODIFIED *
> 
>   28 inodes used (0.02%)
>   14 non-contiguous inodes (50.0%)
>  # of inodes with ind/dind/tind blocks: 15/15/0
>   123417 blocks used (12.19%)
>0 bad blocks
>0 large files
> 
>   16