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  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:	Mon, 22 Jun 2009 09:42:25 -0700
From:	Curt Wohlgemuth <>
To:	Theodore Tso <>
Cc:	ext4 development <>
Subject: Re: RFC PATCH: ext4 no journal corruption with locale-gen

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

   <no journal at all>



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

On Wed, Jun 17, 2009 at 4:46 PM, Theodore Tso<> wrote:
> Hi Curt,
> Thanks for your analysis of the bug.  The reason for the strange logic
> in ext4_set_aops() is because at the moment the code doesn't support
> the combination of data=journalled && delalloc.  That's why it was
> explicitly checking for ext4_should_order_data() and
> ext4_should_writeback_data().
> We have a check for this in ext4_fill_super(), so your patch should be
> safe, since the combination of ext4_should_journal_data &&
> test_opt(inode->i_sb, DELALLOC) should never happen.
> As to your question of whether the nodelalloc and nojournal case
> should really be ext4_journalled_aops, I suspect ext4_writeback_aops
> makes more sense.  I haven't audited all of the code paths to make
> sure they DTRT in the non-journalled case yet, though.
>                                                        - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to
> More majordomo info at
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to
More majordomo info at

Powered by blists - more mailing lists