linux-kernel - Re: [-mm patch] lockdep: possible deadlock in sysfs

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: <200701051613.25882.oliver@neukum.org>
Date:	Fri, 5 Jan 2007 16:13:25 +0100
From:	Oliver Neukum <oliver@...kum.org>
To:	Frederik Deweerdt <deweerdt@...e.fr>
Cc:	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org,
	greg@...ah.com, maneesh@...ibm.com, oliver@...kum.name
Subject: Re: [-mm patch] lockdep: possible deadlock in sysfs

Am Freitag, 5. Januar 2007 13:16 schrieb Frederik Deweerdt:
> On Thu, Jan 04, 2007 at 10:02:00PM -0800, Andrew Morton wrote:
> > 
> > 	ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc3/2.6.20-rc3-mm1/
> > 
> Hi,
> 
> Lockdep issues the following warning:
> [    9.064000] =============================================
> [    9.064000] [ INFO: possible recursive locking detected ]
> [    9.064000] 2.6.20-rc3-mm1 #3
> [    9.064000] ---------------------------------------------
> [    9.064000] init/1 is trying to acquire lock:
> [    9.064000]  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.064000]
> [    9.064000] but task is already holding lock:
> [    9.064000]  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]
> [    9.065000] other info that might help us debug this:
> [    9.065000] 2 locks held by init/1:
> [    9.065000]  #0:  (tty_mutex){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]  #1:  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]
> [    9.065000] stack backtrace:
> [    9.065000]  [<c010390d>] show_trace_log_lvl+0x1a/0x30
> [    9.066000]  [<c0103935>] show_trace+0x12/0x14
> [    9.066000]  [<c0103a2f>] dump_stack+0x16/0x18
> [    9.066000]  [<c0138cb8>] print_deadlock_bug+0xb9/0xc3
> [    9.066000]  [<c0138d17>] check_deadlock+0x55/0x5a
> [    9.066000]  [<c013a953>] __lock_acquire+0x371/0xbf0
> [    9.066000]  [<c013b7a9>] lock_acquire+0x69/0x83
> [    9.066000]  [<c03e6b7e>] __mutex_lock_slowpath+0x75/0x2d1
> [    9.066000]  [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.066000]  [<c01b249c>] sysfs_drop_dentry+0xb1/0x133
> [    9.066000]  [<c01b25d1>] sysfs_hash_and_remove+0xb3/0x142
> [    9.066000]  [<c01b30ed>] sysfs_remove_file+0xd/0x10
> [    9.067000]  [<c02849e0>] device_remove_file+0x23/0x2e
> [    9.067000]  [<c02850b2>] device_del+0x188/0x1e6
> [    9.067000]  [<c028511b>] device_unregister+0xb/0x15
> [    9.067000]  [<c0285318>] device_destroy+0x9c/0xa9
> [    9.067000]  [<c0261431>] vcs_remove_sysfs+0x1c/0x3b
> [    9.067000]  [<c0267a08>] con_close+0x5e/0x6b
> [    9.067000]  [<c02598f2>] release_dev+0x4c4/0x6e5
> [    9.067000]  [<c0259faa>] tty_release+0x12/0x1c
> [    9.067000]  [<c0174872>] __fput+0x177/0x1a0
> [    9.067000]  [<c01746f5>] fput+0x3b/0x41
> [    9.068000]  [<c0172ee1>] filp_close+0x36/0x65
> [    9.068000]  [<c0172f73>] sys_close+0x63/0xa4
> [    9.068000]  [<c0102a96>] sysenter_past_esp+0x5f/0x99
> [    9.068000]  =======================
> 
> This is due to sysfs_hash_and_remove() holding dir->d_inode->i_mutex
> before calling sysfs_drop_dentry() which calls orphan_all_buffers()
> which in turn takes node->i_mutex.
> The following patch solves the problem by defering the buffers orphaning
> after the dir->d_inode->imutex is released. Not sure it's the best
> solution though, better wait for feedback from Maneesh and Oliver.

Hi,

are you sure there's a code path that takes these locks in the reverse order?
I've looked through the code twice and not found any. It doesn't make much
sense to first lock the file and afterwards the directory.

Regarding your patch, it should work, but I don't see the need for it.

	Regards
		Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/