linux-ext4 - Re: [PATCH] fix for consistency errors after crash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTilxxsCSTIFLzg95xooW70SCNfWiDUVqpAT32FbX@mail.gmail.com>
Date:	Tue, 6 Jul 2010 15:00:51 +0200
From:	"Amir G." <amir73il@...rs.sourceforge.net>
To:	Eric Sandeen <sandeen@...hat.com>
Cc:	Ric Wheeler <rwheeler@...hat.com>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] fix for consistency errors after crash

Hi Eric,

I've seen you guys had some open RH bugs on ext3, who all share in
common the "bit already free" error.

This bug I reported can explain many different problems in ext[34].

Essentially, every time there is a kernel crash (or hard reboot)
during delete/truncate of a large file,
it may result in "bit already clear" error after reboot.

The problem is very simple and so is the fix.
I proved the problem with 100% recreation chances using a small patch,
instead of running statistical stress tests.
All I did was to add a print and 10 seconds delay after transaction
restart in ext3_free_branches and reboot > 5 seconds after the
transaction restarts, so that kjournald will have time to commit the
old transaction.
After the reboot, I always get "bit already clear" errors, because the
"half large truncate" transaction is not handled properly.

I did not get any response from ext4 guys so far and since this bug
dates back to ext3,
I was hoping you guys could take a look and put your weight on pushing
the fix upstream.

Thanks,
Amir.

On Wed, Jun 23, 2010 at 9:27 PM, Amir G. <amir73il@...rs.sourceforge.net> wrote:
> Sorry, posting the patch again with a better title.
> This bug needs to be fixed also in Ext3.
> just need to move the ext3_forget() line down to ext3_free_blocks() line.
>
> Amir.
>
> On Wed, Jun 23, 2010 at 10:01 PM, Amir G.
> <amir73il@...rs.sourceforge.net> wrote:
>> Hi,
>>
>> We have experienced bitmap inconsistencies after crash during file
>> delete under heavy load.
>> The crash is not file system related and I the following patch in
>> ext4_free_branches() fixes the recovery problem.
>>
>> If the transaction is restarted and there is a crash before the new
>> transaction is committed,
>> then after recovery, the blocks that this indirect block points to
>> have been freed, but the indirect block itself
>> has not been freed and may still point to some of the free blocks
>> (because of the ext4_forget()).
>>
>> So ext4_forget() should be called inside ext4_free_blocks() to avoid
>> this problem.
>> Are there any consequences to this patch that I am not aware of?
>>
>> Amir.
>>
>> Signed-off-by: Amir Goldstein <amir73il@...rs.sf.net>
>>
>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>> index 42272d6..682e2fa 100644
>> --- a/fs/ext4/inode.c
>> +++ b/fs/ext4/inode.c
>> @@ -4458,6 +4458,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>>  {
>>        ext4_fsblk_t nr;
>>        __le32 *p;
>> +       int     flags;
>>
>>        if (ext4_handle_is_aborted(handle))
>>                return;
>> @@ -4520,7 +4521,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>>                         * revoke records must be emitted *before* clearing
>>                         * this block's bit in the bitmaps.
>>                         */
>> -                       ext4_forget(handle, 1, inode, bh, bh->b_blocknr);
>> +                       flags =
>> EXT4_FREE_BLOCKS_METADATA|EXT4_FREE_BLOCKS_FORGET;
>>
>>                        /*
>>                         * Everything below this this pointer has been
>> @@ -4546,8 +4547,7 @@ static void ext4_free_branches(handle_t *handle,
>> struct inode *inode,
>>                                            blocks_for_truncate(inode));
>>                        }
>>
>> -                       ext4_free_blocks(handle, inode, 0, nr, 1,
>> -                                        EXT4_FREE_BLOCKS_METADATA);
>> +                       ext4_free_blocks(handle, inode, 0, nr, 1, flags);
>>
>>                        if (parent_bh) {
>>                                /*
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html