linux-ext4 - Re: Regular ext4 error warning with HD in USB dock

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Sat, 8 Jan 2011 22:28:01 +0200
From:	Amir Goldstein <amir73il@...il.com>
To:	"Ted Ts'o" <tytso@....edu>
Cc:	Ric Wheeler <rwheeler@...hat.com>,
	Rogier Wolff <R.E.Wolff@...wizard.nl>,
	Con Kolivas <kernel@...ivas.org>, adilger.kernel@...ger.ca,
	linux-ext4@...r.kernel.org
Subject: Re: Regular ext4 error warning with HD in USB dock

On Sat, Jan 8, 2011 at 12:12 AM, Amir Goldstein <amir73il@...il.com> wrote:
> On Fri, Jan 7, 2011 at 11:07 PM, Amir Goldstein <amir73il@...il.com> wrote:
>> On Fri, Jan 7, 2011 at 9:41 PM, Amir Goldstein <amir73il@...il.com> wrote:
>>> On Fri, Jan 7, 2011 at 7:26 AM, Ted Ts'o <tytso@....edu> wrote:
>>>> Am I missing something?  The kernel stores up to 3k worth of data, on
>>>> a 4k block file system.  Whereas e2fsck patch blindly assume 2k worhth
>>>> of data regardless of the block size.  The kernel patch looks ok, but
>>>> the e2fsprogs patch seems badly broken....
>>
>> So it's not badly broken, it copies blocksize-2K, which is clumsily
>> written like this:
>> +       int len = ctx->fs->blocksize - 2*SUPERBLOCK_OFFSET;
>>
>> After that, only 4K block and 8K block will have a leftover,
>> which will be copied from journal sb+1K to ext4 sb+1K.
>> Yes, 2K blocks - no message buffer for you!
>>
>> The reason I am only copying 2K and throwing the extra K is this:
>> print_message_buffer() is called from check_super_block(),
>> *after* journal recovery, which was executed either by e2fsck itself
>> or (and more likely in a errors=remount-ro situation) by ext4
>> on read-only mount.
>> In the latter case, the extra K must be discarded, so I saw no reason
>> to write special code for the first case.
>> Neither did I find a good reason to complicate the recording code
>> and limit it to record only blocksize-2K.
>>
>>
>
> Ted,
>
> I have a suggestion how to use the wasted extra K.
>
> As I pointed out in the past, the first/last_error_xxx statistics are most
> likely to be lost in errors=panic and errors=remount-ro (journal
> recovery will override super block)
> If you record this information in the last K of journal sb (even copy
> the entire ext4 sb),
> you can then override ext4 sb with the most up-to-date error stats
> after journal recovery.
>
> Amir.
>

Ted,

I just realize you did implement save&update of super block s_error_xxx fields.
However, I wonder if it is not a bug to call ext4_commit_super() from
save_error_info()
to commit the s_error_xxx fields in the first place.

The super block buffer is most likely participating in the current
transaction and should
not be committed to disk.

The alleged bug is likely to be hidden by the fact that the super
block has most likely
participated also in the last committed transaction, so a valid
version of it will most
likely override the invalid version.

I can imagine there could be a corner case, though, when committing
the super block
a head of transaction commit will result in inconsistencies.

Am I missing something?

Amir.
--
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