linux-ext4 - Re: [PATCH 2/4] ext3: Fix possible deadlock between ext3_truncate() and ext3_get

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: <20090817144524.GA1611@duck.novell.com>
Date:	Mon, 17 Aug 2009 16:45:24 +0200
From:	Jan Kara <jack@...e.cz>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Jan Kara <jack@...e.cz>, linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH 2/4] ext3: Fix possible deadlock between
	ext3_truncate() and ext3_get_blocks()

On Fri 14-08-09 16:41:05, Andrew Morton wrote:
> On Fri, 14 Aug 2009 14:26:10 +0200
> Jan Kara <jack@...e.cz> wrote:
> 
> > During truncate we are sometimes forced to start a new transaction as the
> > amount of blocks to be journaled is both quite large and hard to predict. So
> > far we restarted a transaction while holding truncate_mutex and that violates
> > lock ordering because truncate_mutex ranks below transaction start (and it
> > can lead to a real deadlock with ext3_get_blocks() allocating new blocks
> > from ext3_writepage()).
> > 
> > Luckily, the problem is easy to fix: We just drop the truncate_mutex before
> > restarting the transaction and acquire it afterwards. We are safe to do this as
> > by the time ext3_truncate() is called, all the page cache for the truncated
> > part of the file is dropped and so writepage() cannot come and allocate new
> > blocks in the part of the file we are truncating. The rest of writers is
> > stopped by us holding i_mutex.
> 
> For ext2 we have the comment:
> 
> 	/*
> 	 * truncate_mutex is for serialising ext2_truncate() against
> 	 * ext2_getblock().  It also protects the internals of the inode's
> 	 * reservation data structures: ext2_reserve_window and
> 	 * ext2_reserve_window_node.
> 	 */
> 
> does truncate_mutex also protect ext3's reservation data?  If so, is
> that impacted by this patch?
  That's a good question: ext3 seems to use truncate_mutex to guard
reservation internals as well. Currently, truncate calls
ext3_discard_reservation() in the end of truncate. That is slightly
suboptimal when we can drop truncate_mutex during truncate as that can lead
to allocation from reservation window which is not well placed wrt the new
file end. Otherwise I don't think there's any issue since reservation is
just an interval of blocks we'd like to allocate from so it's not bound to
blocks allocated to the inode in any way.
  I'll move ext3_discard_reservation() to the beginning of the truncate
to fix the above issue. Thanks for the comment. Attached is a new version
of the patch.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR

View attachment "0001-ext3-Fix-possible-deadlock-between-ext3_truncate.patch" of type "text/x-patch" (3692 bytes)