linux-ext4 - Re: ext4: Can we talk about bforget() and metadata blocks

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: <20090912150036.GA13906@skywalker.linux.vnet.ibm.com>
Date:	Sat, 12 Sep 2009 20:30:36 +0530
From:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To:	Theodore Tso <tytso@....edu>
Cc:	Curt Wohlgemuth <curtw@...gle.com>, linux-ext4@...r.kernel.org
Subject: Re: ext4: Can we talk about bforget() and metadata blocks

On Thu, Sep 10, 2009 at 02:58:26PM -0400, Theodore Tso wrote:
> On Thu, Sep 10, 2009 at 09:54:35PM +0530, Aneesh Kumar K.V wrote:
> > 
> > But how would it work for fsync ? I mean 
> > 
> > I would expect for no journal mode ext4_sync_file  should be doing
> > simple_fsync(). That should be forcing the metadata buffer_heads
> > via sync_mapping_buffers. And if we reuse these meta buffers we
> > drop them the inode->mapping->private_list using bforget.
> > 
> > But I don't see any of the above in code
> 
> Aneesh, you're addressing a different problem than the one that Curt
> were trying to deal with this patch.  The problem we are worry about
> is one where an inode's extent tree or indirect blocks are modified
> right before the inode is deleted, and then one or more of those
> metadata blocks get reallocated and written right away (most likely
> this will happen via an O_DIRECT write), and then, because we didn't
> use bforget(), the dirty metadata block in the buffer cache would get
> written out, overwriting the O_DIRECT block.
> 
> What you're worrying about, is a different issue.  You're concerned
> about the fact that since we are not associating an inode's extent
> tree or indirect blocks with the inode, those blocks won't get forced
> out to disk on an fsync() in ext4 no-journal mode.  This may not be a
> big deal for applications which expect to recover from an unclean
> using mke2fs (and thus probably don't use fsync in any case), but
> here's a patch to deal with the problem you've raised.
> 
>        	       	       	    		       - Ted
> 
> commit 417cf58253fbf3e36df7b3aca11c120e8367f5e6
> Author: Theodore Ts'o <tytso@....edu>
> Date:   Thu Sep 10 14:58:02 2009 -0400
> 
>     ext4: Assure that metadata blocks are written during fsync in no journal mode
>     
>     When there is no journal present, we must attach buffer heads
>     associated with extent tree and indirect blocks to the inode's
>     mapping->private_list so that fsync() will write out the inode's
>     metadata blocks.  This is done via mark_buffer_dirty_inode().
>     
>     Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> 
> diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
> index ecb9ca4..6a94099 100644
> --- a/fs/ext4/ext4_jbd2.c
> +++ b/fs/ext4/ext4_jbd2.c
> @@ -89,7 +89,10 @@ int __ext4_handle_dirty_metadata(const char *where, handle_t *handle,
>  			ext4_journal_abort_handle(where, __func__, bh,
>  						  handle, err);
>  	} else {
> -		mark_buffer_dirty(bh);
> +		if (inode && bh)
> +			mark_buffer_dirty_inode(bh, inode);
> +		else
> +			mark_buffer_dirty(bh);
>  		if (inode && inode_needs_sync(inode)) {
>  			sync_dirty_buffer(bh);
>  			if (buffer_req(bh) && !buffer_uptodate(bh)) {
> 


This does add the meta data buffer_head to the inode->mapping->private_list.
But ext4_sync_file is not writing them. I guess we need to call sync_mapping_buffers
for no-journal mode in ext4_sync_file

-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