linux-ext4 - Re: [PATCH] ext4: Fix circular locking dependency with fallocate and touch.

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] [day] [month] [year] [list]

Date:	Fri, 15 Feb 2008 12:52:29 -0500
From:	Theodore Tso <tytso@....EDU>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	cmm@...ibm.com, linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext4: Fix circular locking dependency with fallocate
	and touch.

On Tue, Feb 12, 2008 at 02:45:35PM +0530, Aneesh Kumar K.V wrote:
> In order to prevent a circular locking dependency when an ext4_create
> operation is racing with an ext4_fallocate, we acquire and release
> i_data_sem for each multiblock request and use i_mutex to
> prevent writes and truncates during the complete fallocate operation.
> 
> 
> =======================================================
> [ INFO: possible circular locking dependency detected ]
> 2.6.25-rc1 #4
> -------------------------------------------------------
> touch/2347 is trying to acquire lock:
>  (&ei->i_data_sem){----}, at: [<c01cffed>] ext4_get_blocks_wrap+0x21/0xca
> 
> but task is already holding lock:
>  (jbd2_handle){--..}, at: [<c01ee43c>] jbd2_journal_start+0xce/0xf0
> 
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> ...

I replaced the patch description with a more succint and to-the-point
changelog message:

   ext4_fallocate() was trying to acquire i_data_sem outside of
   jbd2_start_transaction/jbd2_journal_stop, which violates ext4's locking
   hierarchy.  So we take i_mutex to prevent writes and truncates during
   the complete fallocate operation, and use ext4_get_block_wrap() which
   acquires and releases i_data_sem for each block allocation.

BTW, a good thing for us to do would be to document our current
locking hierarchy: what each lock protects, and in what order locks
should be taken.  

This makes it easier for people to notice problems by reviewing code
(as opposed to trusting that lockdep will turn up problems).  It also
is a good idea so that as we start to do performance testing and start
noticing inappropriate lock hold times, we can look at our locking
hierarchy design and see if it makes sense, and how to change it to
improve performance.

Regards,

						- Ted
-
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