linux-ext4 - Re: jbd2_handle and i_data_sem circular locking dependency detected

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]

Date:	Mon, 4 Feb 2008 10:23:16 -0500
From:	Josef Bacik <jbacik@...hat.com>
To:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc:	Jan Kara <jack@...e.cz>, Theodore Tso <tytso@....edu>,
	Andreas Dilger <adilger@....com>,
	Mingming Cao <cmm@...ibm.com>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: jbd2_handle and i_data_sem circular locking dependency detected

On Monday 04 February 2008 5:12:28 am Aneesh Kumar K.V wrote:
> Hi,
>
> This is with the new ext3 -> ext4 migrate code added. The recently added
> lockdep for jbd2 helped to find this out. We want to hold the i_data_sem
> on the ext3 inode during migration to prevent walking the ext3 inode
> when it is being converted to ext4 format. Also we want to avoid
> file truncation and new blocks being added while converting to ext4.
> Also we dont want to reserve large number of credits for journal.
> Any idea how to fix this ?
>

Hello,

Seems you should be taking the i_data_sem after starting the journal in 
ext4_ext_migrate.  I haven't looked at this stuff too much, but everywhere I 
see i_data_sem taken its within a journal_start/journal_stop.  Here's the 
patch.  Let me know if I'm way off btw, like I said I'm new to this :).  Thank 
you,

Josef

Index: linux-2.6/fs/ext4/migrate.c
===================================================================
--- linux-2.6.orig/fs/ext4/migrate.c
+++ linux-2.6/fs/ext4/migrate.c
@@ -414,7 +414,6 @@ int ext4_ext_migrate(struct inode *inode
 	if ((EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
 		return -EINVAL;
 
-	down_write(&EXT4_I(inode)->i_data_sem);
 	handle = ext4_journal_start(inode,
 					EXT4_DATA_TRANS_BLOCKS(inode->i_sb) +
 					EXT4_INDEX_EXTRA_TRANS_BLOCKS + 3 +
@@ -424,6 +423,9 @@ int ext4_ext_migrate(struct inode *inode
 		retval = PTR_ERR(handle);
 		goto err_out;
 	}
+
+	down_write(&EXT4_I(inode)->i_data_sem);
+
 	tmp_inode = ext4_new_inode(handle,
 				inode->i_sb->s_root->d_inode,
 				S_IFREG);
@@ -549,10 +551,10 @@ err_out:
 	 */
 	tmp_inode->i_nlink = 0;
 
-	ext4_journal_stop(handle);
-
 	up_write(&EXT4_I(inode)->i_data_sem);
 
+	ext4_journal_stop(handle);
+
 	if (tmp_inode)
 		iput(tmp_inode);
 




-
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