linux-ext4 - Re: RFC PATCH: ext4 no journal corruption with locale-gen

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]

Message-ID: <6601abe90906221001u15f370e6vcb36a02d54d2f8c3@mail.gmail.com>
Date:	Mon, 22 Jun 2009 10:01:39 -0700
From:	Curt Wohlgemuth <curtw@...gle.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	Theodore Tso <tytso@....edu>,
	ext4 development <linux-ext4@...r.kernel.org>
Subject: Re: RFC PATCH: ext4 no journal corruption with locale-gen

On Mon, Jun 22, 2009 at 9:56 AM, Aneesh Kumar
K.V<aneesh.kumar@...ux.vnet.ibm.com> wrote:
> On Mon, Jun 22, 2009 at 09:42:25AM -0700, Curt Wohlgemuth wrote:
>> Hi Ted:
>>
>> I think the following patch is sufficient.  It explicitly sets the aops to
>> ext4_writeback_aops if there is no delayed allocation and no journal.
>>
>> I tested the locale-gen example with all combinations of
>>
>>    data=writeback
>>    data=ordered
>>    data=journal
>>    <no journal at all>
>>
>> and
>>
>>    delalloc
>>    nodelalloc
>>
>> and it works correctly now.  The paths for writeback seem fine to me for an
>> inode w/o a journal.
>>
>>
>>        Signed-off-by: Curt Wohlgemuth <curtw@...gle.com>
>> ---
>> --- 2.6.26/fs/ext4/inode.c.orig       2009-06-09 20:05:27.000000000 -0700
>> +++ 2.6.26/fs/ext4/inode.c    2009-06-22 08:55:13.000000000 -0700
>> @@ -3442,15 +3442,12 @@ static const struct address_space_operat
>>
>>  void ext4_set_aops(struct inode *inode)
>>  {
>> -     if (ext4_should_order_data(inode) &&
>> -             test_opt(inode->i_sb, DELALLOC))
>> +     if (test_opt(inode->i_sb, DELALLOC))
>
> This change is not related to the fix right ?
>
>>               inode->i_mapping->a_ops = &ext4_da_aops;
>>       else if (ext4_should_order_data(inode))
>>               inode->i_mapping->a_ops = &ext4_ordered_aops;
>> -     else if (ext4_should_writeback_data(inode) &&
>> -              test_opt(inode->i_sb, DELALLOC))
>> -             inode->i_mapping->a_ops = &ext4_da_aops;
>> -     else if (ext4_should_writeback_data(inode))
>> +     else if (ext4_should_writeback_data(inode) ||
>> +                              EXT4_JOURNAL(inode) == NULL)
>>               inode->i_mapping->a_ops = &ext4_writeback_aops;
>
> Can you send a patch with this hunk alone. The previous one is not
> related to the fix right ?

You need them both.  We want all inodes with delayed allocation to use
ext4_da_aops.  The fall-through cases are just for nodelalloc.

Curt

>
>
>>       else
>>               inode->i_mapping->a_ops = &ext4_journalled_aops;
>>
>>
>
> -aneesh
>
--
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