2009-09-01 21:43:16

by Eric Sandeen

[permalink] [raw]
Subject: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

ext2fs_open2() was only looking at s_blocks_count, and
when it wrapped to a low number, it was failing the test of:

fs->super->s_first_data_block >= fs->super->s_blocks_count

which made the superblock look corrupt.

Signed-off-by: Eric Sandeen <[email protected]>
---

Patch is against the pu branch

Index: e2fsprogs/lib/ext2fs/openfs.c
===================================================================
--- e2fsprogs.orig/lib/ext2fs/openfs.c
+++ e2fsprogs/lib/ext2fs/openfs.c
@@ -288,7 +288,7 @@ errcode_t ext2fs_open2(const char *name,
blocks_per_group > EXT2_MAX_BLOCKS_PER_GROUP(fs->super) ||
fs->inode_blocks_per_group > EXT2_MAX_INODES_PER_GROUP(fs->super) ||
EXT2_DESC_PER_BLOCK(fs->super) == 0 ||
- fs->super->s_first_data_block >= fs->super->s_blocks_count) {
+ fs->super->s_first_data_block >= ext2fs_blocks_count(fs->super)) {
retval = EXT2_ET_CORRUPT_SUPERBLOCK;
goto cleanup;
}



2009-09-02 05:59:45

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Sep 01, 2009 16:43 -0500, Eric Sandeen wrote:
> ext2fs_open2() was only looking at s_blocks_count, and
> when it wrapped to a low number, it was failing the test of:
>
> fs->super->s_first_data_block >= fs->super->s_blocks_count
>
> which made the superblock look corrupt.

Is this the source of the "e2fsck is finding bad checksums" problem?

> Patch is against the pu branch
>
> Index: e2fsprogs/lib/ext2fs/openfs.c
> ===================================================================
> --- e2fsprogs.orig/lib/ext2fs/openfs.c
> +++ e2fsprogs/lib/ext2fs/openfs.c
> @@ -288,7 +288,7 @@ errcode_t ext2fs_open2(const char *name,
> blocks_per_group > EXT2_MAX_BLOCKS_PER_GROUP(fs->super) ||
> fs->inode_blocks_per_group > EXT2_MAX_INODES_PER_GROUP(fs->super) ||
> EXT2_DESC_PER_BLOCK(fs->super) == 0 ||
> - fs->super->s_first_data_block >= fs->super->s_blocks_count) {
> + fs->super->s_first_data_block >= ext2fs_blocks_count(fs->super)) {

I would strongly suggest to replace the declaration of "s_blocks_count"
with "s_blocks_count_lo" (and similar for every other split value), so
that we catch all instances of this type of bug.

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


2009-09-02 06:12:28

by Justin Maggard

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Tue, Sep 1, 2009 at 10:59 PM, Andreas Dilger<[email protected]> wrote:
> On Sep 01, 2009 ?16:43 -0500, Eric Sandeen wrote:
>> ext2fs_open2() was only looking at s_blocks_count, and
>> when it wrapped to a low number, it was failing the test of:
>>
>> ? fs->super->s_first_data_block >= fs->super->s_blocks_count
>>
>> which made the superblock look corrupt.
>
> Is this the source of the "e2fsck is finding bad checksums" problem?

I applied this earlier today, and it didn't appear to help in my test case.

-Justin

2009-09-02 16:43:13

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Justin Maggard wrote:
> On Tue, Sep 1, 2009 at 10:59 PM, Andreas Dilger<[email protected]> wrote:
>> On Sep 01, 2009 16:43 -0500, Eric Sandeen wrote:
>>> ext2fs_open2() was only looking at s_blocks_count, and
>>> when it wrapped to a low number, it was failing the test of:
>>>
>>> fs->super->s_first_data_block >= fs->super->s_blocks_count
>>>
>>> which made the superblock look corrupt.
>> Is this the source of the "e2fsck is finding bad checksums" problem?
>
> I applied this earlier today, and it didn't appear to help in my test case.
>
> -Justin

Nah, didn't expect it to, but I'm working towards that. Just have to
whack down the bugs between where I am, and your bug ;)

-Eric

2009-09-02 21:02:19

by Nick Dokos

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

> Justin Maggard wrote:
> > On Tue, Sep 1, 2009 at 10:59 PM, Andreas Dilger<[email protected]> wrote:
> >> On Sep 01, 2009 16:43 -0500, Eric Sandeen wrote:
> >>> ext2fs_open2() was only looking at s_blocks_count, and
> >>> when it wrapped to a low number, it was failing the test of:
> >>>
> >>> fs->super->s_first_data_block >= fs->super->s_blocks_count
> >>>
> >>> which made the superblock look corrupt.
> >> Is this the source of the "e2fsck is finding bad checksums" problem?
> >
> > I applied this earlier today, and it didn't appear to help in my test case.
> >
> > -Justin
>
> Nah, didn't expect it to, but I'm working towards that. Just have to
> whack down the bugs between where I am, and your bug ;)
>
> -Eric

The following patch fixes a problem I think, but I'm not sure whether it
resolves Justin's problem. I'm running a test, but I thought I'd send it
out for people to try and/or comment on. Let me know of any problems.

Thanks,
Nick

>From e8c790ab8a0f06b4c89a5b6ddba2a36f033c742c Mon Sep 17 00:00:00 2001
From: Nick Dokos <[email protected]>
Date: Wed, 2 Sep 2009 16:52:09 -0400
Subject: [PATCH] Fix counting routines in blknum.c to take/return __u32 counts.

Several routines in lib/ext2fs/blknum.c:

ext2fs_bg_free_blocks_count()
ext2fs_bg_free_inodes_count()
ext2fs_bg_used_dirs_count()
ext2fs_bg_itable_unused()

and their _set() counterparts, operate as if they are dealing with
blk64_t quantities, but they should be dealing with __u32 counts
instead.

Signed-off-by: Nick Dokos <[email protected]>
---
lib/ext2fs/blknum.c | 48 ++++++++++++++++++++++++------------------------
lib/ext2fs/ext2fs.h | 16 ++++++++--------
2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/lib/ext2fs/blknum.c b/lib/ext2fs/blknum.c
index fd56d53..1f183f4 100644
--- a/lib/ext2fs/blknum.c
+++ b/lib/ext2fs/blknum.c
@@ -274,7 +274,7 @@ void ext2fs_inode_table_loc_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
/*
* Return the free blocks count of a group
*/
-blk64_t ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group)
+__u32 ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
@@ -283,7 +283,7 @@ blk64_t ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group)
return gdp->bg_free_blocks_count |
(fs->super->s_feature_incompat
& EXT4_FEATURE_INCOMPAT_64BIT ?
- (__u64) gdp->bg_free_blocks_count_hi << 32 : 0);
+ (__u32) gdp->bg_free_blocks_count_hi << 16 : 0);
}

return fs->group_desc[group].bg_free_blocks_count;
@@ -292,22 +292,22 @@ blk64_t ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group)
/*
* Set the free blocks count of a group
*/
-void ext2fs_bg_free_blocks_count_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
+void ext2fs_bg_free_blocks_count_set(ext2_filsys fs, dgrp_t group, __u32 n)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
gdp = (struct ext4_group_desc *) (fs->group_desc) + group;
- gdp->bg_free_blocks_count = blk;
+ gdp->bg_free_blocks_count = n;
if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
- gdp->bg_free_blocks_count_hi = (__u64) blk >> 32;
+ gdp->bg_free_blocks_count_hi = (__u32) n >> 16;
} else
- fs->group_desc[group].bg_free_blocks_count = blk;
+ fs->group_desc[group].bg_free_blocks_count = n;
}

/*
* Return the free inodes count of a group
*/
-blk64_t ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group)
+__u32 ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
@@ -316,7 +316,7 @@ blk64_t ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group)
return gdp->bg_free_inodes_count |
(fs->super->s_feature_incompat
& EXT4_FEATURE_INCOMPAT_64BIT ?
- (__u64) gdp->bg_free_inodes_count_hi << 32 : 0);
+ (__u32) gdp->bg_free_inodes_count_hi << 16 : 0);
}

return fs->group_desc[group].bg_free_inodes_count;
@@ -325,22 +325,22 @@ blk64_t ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group)
/*
* Set the free inodes count of a group
*/
-void ext2fs_bg_free_inodes_count_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
+void ext2fs_bg_free_inodes_count_set(ext2_filsys fs, dgrp_t group, __u32 n)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
gdp = (struct ext4_group_desc *) (fs->group_desc) + group;
- gdp->bg_free_inodes_count = blk;
+ gdp->bg_free_inodes_count = n;
if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
- gdp->bg_free_inodes_count_hi = (__u64) blk >> 32;
+ gdp->bg_free_inodes_count_hi = (__u32) n >> 16;
} else
- fs->group_desc[group].bg_free_inodes_count = blk;
+ fs->group_desc[group].bg_free_inodes_count = n;
}

/*
* Return the used dirs count of a group
*/
-blk64_t ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group)
+__u32 ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
@@ -349,7 +349,7 @@ blk64_t ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group)
return gdp->bg_used_dirs_count |
(fs->super->s_feature_incompat
& EXT4_FEATURE_INCOMPAT_64BIT ?
- (__u64) gdp->bg_used_dirs_count_hi << 32 : 0);
+ (__u32) gdp->bg_used_dirs_count_hi << 16 : 0);
}

return fs->group_desc[group].bg_used_dirs_count;
@@ -358,22 +358,22 @@ blk64_t ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group)
/*
* Set the used dirs count of a group
*/
-void ext2fs_bg_used_dirs_count_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
+void ext2fs_bg_used_dirs_count_set(ext2_filsys fs, dgrp_t group, __u32 n)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
gdp = (struct ext4_group_desc *) (fs->group_desc) + group;
- gdp->bg_used_dirs_count = blk;
+ gdp->bg_used_dirs_count = n;
if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
- gdp->bg_used_dirs_count_hi = (__u64) blk >> 32;
+ gdp->bg_used_dirs_count_hi = (__u32) n >> 16;
} else
- fs->group_desc[group].bg_used_dirs_count = blk;
+ fs->group_desc[group].bg_used_dirs_count = n;
}

/*
* Return the unused inodes count of a group
*/
-blk64_t ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group)
+__u32 ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
@@ -382,7 +382,7 @@ blk64_t ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group)
return gdp->bg_itable_unused |
(fs->super->s_feature_incompat
& EXT4_FEATURE_INCOMPAT_64BIT ?
- (__u64) gdp->bg_itable_unused_hi << 32 : 0);
+ (__u32) gdp->bg_itable_unused_hi << 16 : 0);
}

return fs->group_desc[group].bg_itable_unused;
@@ -391,16 +391,16 @@ blk64_t ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group)
/*
* Set the unused inodes count of a group
*/
-void ext2fs_bg_itable_unused_set(ext2_filsys fs, dgrp_t group, blk64_t blk)
+void ext2fs_bg_itable_unused_set(ext2_filsys fs, dgrp_t group, __u32 n)
{
if (fs->super->s_desc_size >= EXT2_MIN_DESC_SIZE_64BIT) {
struct ext4_group_desc *gdp;
gdp = (struct ext4_group_desc *) (fs->group_desc) + group;
- gdp->bg_itable_unused = blk;
+ gdp->bg_itable_unused = n;
if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
- gdp->bg_itable_unused_hi = (__u64) blk >> 32;
+ gdp->bg_itable_unused_hi = (__u32) n >> 16;
} else
- fs->group_desc[group].bg_itable_unused = blk;
+ fs->group_desc[group].bg_itable_unused = n;
}

/*
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index c564acf..4c5313c 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -763,18 +763,18 @@ extern void ext2fs_inode_bitmap_loc_set(ext2_filsys fs, dgrp_t group,
extern blk64_t ext2fs_inode_table_loc(ext2_filsys fs, dgrp_t group);
extern void ext2fs_inode_table_loc_set(ext2_filsys fs, dgrp_t group,
blk64_t blk);
-extern blk64_t ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group);
+extern __u32 ext2fs_bg_free_blocks_count(ext2_filsys fs, dgrp_t group);
extern void ext2fs_bg_free_blocks_count_set(ext2_filsys fs, dgrp_t group,
- blk64_t blk);
-extern blk64_t ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group);
+ __u32 n);
+extern __u32 ext2fs_bg_free_inodes_count(ext2_filsys fs, dgrp_t group);
extern void ext2fs_bg_free_inodes_count_set(ext2_filsys fs, dgrp_t group,
- blk64_t blk);
-extern blk64_t ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group);
+ __u32 n);
+extern __u32 ext2fs_bg_used_dirs_count(ext2_filsys fs, dgrp_t group);
extern void ext2fs_bg_used_dirs_count_set(ext2_filsys fs, dgrp_t group,
- blk64_t blk);
-extern blk64_t ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group);
+ __u32 n);
+extern __u32 ext2fs_bg_itable_unused(ext2_filsys fs, dgrp_t group);
extern void ext2fs_bg_itable_unused_set(ext2_filsys fs, dgrp_t group,
- blk64_t blk);
+ __u32 n);
extern __u16 ext2fs_bg_flags(ext2_filsys fs, dgrp_t group);
extern void ext2fs_bg_flags_set(ext2_filsys fs, dgrp_t group, __u16 bg_flags);
extern void ext2fs_bg_flags_clear(ext2_filsys fs, dgrp_t group,
--
1.6.0.6


2009-09-02 21:28:21

by Justin Maggard

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
> The following patch fixes a problem I think, but I'm not sure whether it
> resolves Justin's problem. I'm running a test, but I thought I'd send it
> out for people to try and/or comment on. Let me know of any problems.

Just finished trying with that patch, but it looks like it doesn't
resolve my issue either.

-Justin

2009-09-02 21:31:33

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Justin Maggard wrote:
> On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
>> The following patch fixes a problem I think, but I'm not sure whether it
>> resolves Justin's problem. I'm running a test, but I thought I'd send it
>> out for people to try and/or comment on. Let me know of any problems.
>
> Just finished trying with that patch, but it looks like it doesn't
> resolve my issue either.
>
> -Justin

Ok, I had a thinko in there, steered astray by a weird function
arguments; V2 coming in a moment :)

-Eric

2009-09-02 21:37:27

by Nick Dokos

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Justin Maggard <[email protected]> wrote:

> On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
> > The following patch fixes a problem I think, but I'm not sure whether it
> > resolves Justin's problem. I'm running a test, but I thought I'd send it
> > out for people to try and/or comment on. Let me know of any problems.
>
> Just finished trying with that patch, but it looks like it doesn't
> resolve my issue either.
>

Yup, it didn't pass my test either.

Thanks,
Nick

2009-09-02 21:43:32

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Nick Dokos wrote:
> Justin Maggard <[email protected]> wrote:
>
>> On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
>>> The following patch fixes a problem I think, but I'm not sure whether it
>>> resolves Justin's problem. I'm running a test, but I thought I'd send it
>>> out for people to try and/or comment on. Let me know of any problems.
>> Just finished trying with that patch, but it looks like it doesn't
>> resolve my issue either.
>>
>
> Yup, it didn't pass my test either.

You guys are still getting bad checksums?

-Eric

2009-09-02 21:45:35

by Justin Maggard

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Wed, Sep 2, 2009 at 2:43 PM, Eric Sandeen<[email protected]> wrote:
> Nick Dokos wrote:
>> Justin Maggard <[email protected]> wrote:
>>
>>> On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
>>>> The following patch fixes a problem I think, but I'm not sure whether it
>>>> resolves Justin's problem. I'm running a test, but I thought I'd send it
>>>> out for people to try and/or comment on. Let me know of any problems.
>>> Just finished trying with that patch, but it looks like it doesn't
>>> resolve my issue either.
>>>
>>
>> Yup, it didn't pass my test either.
>
> You guys are still getting bad checksums?

Yeah, I am.

2009-09-02 22:28:08

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Wed, Sep 02, 2009 at 05:02:22PM -0400, Nick Dokos wrote:
> The following patch fixes a problem I think, but I'm not sure whether it
> resolves Justin's problem. I'm running a test, but I thought I'd send it
> out for people to try and/or comment on. Let me know of any problems.

Ouch. Thanks for sending this; I'm surprised people hadn't run into
massive problems due to this patch earlier.

Acked-by: "Theodore Ts'o" <[email protected]>

- Ted

2009-09-02 22:33:18

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Justin Maggard wrote:
> On Wed, Sep 2, 2009 at 2:43 PM, Eric Sandeen<[email protected]> wrote:
>> Nick Dokos wrote:
>>> Justin Maggard <[email protected]> wrote:
>>>
>>>> On Wed, Sep 2, 2009 at 2:02 PM, Nick Dokos<[email protected]> wrote:
>>>>> The following patch fixes a problem I think, but I'm not sure whether it
>>>>> resolves Justin's problem. I'm running a test, but I thought I'd send it
>>>>> out for people to try and/or comment on. Let me know of any problems.
>>>> Just finished trying with that patch, but it looks like it doesn't
>>>> resolve my issue either.
>>>>
>>> Yup, it didn't pass my test either.
>> You guys are still getting bad checksums?
>
> Yeah, I am.

Oh, sorry, all the other bugs gave me a head-fake, and I forgot the
original problem of -fsck- corrupting the checksums. :) I had a simple
mkdir giving me the corruptions. Ok, on to that.

-Eric

2009-09-02 22:55:20

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Sep 02, 2009 17:33 -0500, Eric Sandeen wrote:
> Justin Maggard wrote:
> > On Wed, Sep 2, 2009 at 2:43 PM, Eric Sandeen<[email protected]> wrote:
> >> You guys are still getting bad checksums?
> >
> > Yeah, I am.
>
> Oh, sorry, all the other bugs gave me a head-fake, and I forgot the
> original problem of -fsck- corrupting the checksums. :) I had a simple
> mkdir giving me the corruptions. Ok, on to that.

I found the source of the checksum error last night - the reserved bytes
in the 64-bit group descriptor are not zero after the e2fsck is run.
It should be pretty easy to run e2fsck under gdb and put a hardware watch
on those bytes to see who twiddles them.

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


2009-09-02 23:12:19

by Nick Dokos

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

> On Wed, Sep 02, 2009 at 05:02:22PM -0400, Nick Dokos wrote:
> > The following patch fixes a problem I think, but I'm not sure whether it
> > resolves Justin's problem. I'm running a test, but I thought I'd send it
> > out for people to try and/or comment on. Let me know of any problems.
>
> Ouch. Thanks for sending this; I'm surprised people hadn't run into
> massive problems due to this patch earlier.
>

Most of the time those counts fit in 16 bits (they are per block group,
is that right?) - at least with default values: you'd have to make a
file system with larger block groups to see the problem, I think. So
it's not as painful as it first sounds.

Nick

2009-09-03 02:41:12

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

Andreas Dilger wrote:
> On Sep 02, 2009 17:33 -0500, Eric Sandeen wrote:
>> Justin Maggard wrote:
>>> On Wed, Sep 2, 2009 at 2:43 PM, Eric Sandeen<[email protected]> wrote:
>>>> You guys are still getting bad checksums?
>>> Yeah, I am.
>> Oh, sorry, all the other bugs gave me a head-fake, and I forgot the
>> original problem of -fsck- corrupting the checksums. :) I had a simple
>> mkdir giving me the corruptions. Ok, on to that.
>
> I found the source of the checksum error last night - the reserved bytes
> in the 64-bit group descriptor are not zero after the e2fsck is run.
> It should be pretty easy to run e2fsck under gdb and put a hardware watch
> on those bytes to see who twiddles them.

Thanks, I saw that email a bit late today, will take a look.

-Eric

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


2009-09-06 16:30:40

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] libext2fs: use ext2fs_blocks_count() in ext2fs_open2()

On Tue, Sep 01, 2009 at 04:43:16PM -0500, Eric Sandeen wrote:
> ext2fs_open2() was only looking at s_blocks_count, and
> when it wrapped to a low number, it was failing the test of:
>
> fs->super->s_first_data_block >= fs->super->s_blocks_count
>
> which made the superblock look corrupt.
>
> Signed-off-by: Eric Sandeen <[email protected]>

Added to the e2fsprogs 64-bit patch set / pu branch.

- Ted

2009-09-06 16:37:08

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] Fix counting routines in blknum.c to take/return __u32 counts.

On Wed, Sep 02, 2009 at 05:02:22PM -0400, Nick Dokos wrote:
> From e8c790ab8a0f06b4c89a5b6ddba2a36f033c742c Mon Sep 17 00:00:00 2001
> From: Nick Dokos <[email protected]>
> Date: Wed, 2 Sep 2009 16:52:09 -0400
> Subject: [PATCH] Fix counting routines in blknum.c to take/return __u32 counts.
>
> Several routines in lib/ext2fs/blknum.c:
>
> ext2fs_bg_free_blocks_count()
> ext2fs_bg_free_inodes_count()
> ext2fs_bg_used_dirs_count()
> ext2fs_bg_itable_unused()
>
> and their _set() counterparts, operate as if they are dealing with
> blk64_t quantities, but they should be dealing with __u32 counts
> instead.
>
> Signed-off-by: Nick Dokos <[email protected]>

Added to the e2fsprogs pu branch / 64-bit patch set.

BTW, note that this patch by itself caused the regression test suite
to explode spectactularly, since there was a number of printf-style
format statements that had to be changed so that dumpe2fs, debugfs
et. al, would work correctly after changing the above-mentioned
routines to return 32-bit values instead of 64-bit values.

I've fixed this before adding it to the e2fsprogs patch set.

Even though we do need to add more 64-bit tests, I do appreciate it if
people could test their patches using "make check" before sending
patches them to the ext4 list.

- Ted