linux-kernel - Re: [BUG 3.13.0-rc6] reiserfs possible circular locking dependency

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: <CA+55aFxVs_aGnMdcTck3C7AayFrjX=GrFSwmJwiC9q3-3wySgw@mail.gmail.com>
Date:	Fri, 3 Jan 2014 11:46:29 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Knut Petersen <Knut_Petersen@...nline.de>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Jeff Mahoney <jeffm@...e.com>, reiserfs-devel@...r.kernel.org
Subject: Re: [BUG 3.13.0-rc6] reiserfs possible circular locking dependency

On Fri, Jan 3, 2014 at 11:16 AM, Knut Petersen
<Knut_Petersen@...nline.de> wrote:
> Rebooting after a power failure on an openSuSE 13.1 system
> with kernel 3.13.0-rc6 triggered the attached lockdep warning.

Hmm. It seems to be that the *normal* sequence should be:

 - get i_mutex, call lookup, which gets sbi->lock (reiserfs_write_lock)

but in the mounting path, we have special circumstances.

That finish_unfinished() function does

 - reiserfs_write_lock_nested() .
 - remove_save_link
 - iput(inode) with the write lock held

and that can apparently end up taking i_mutex in open_xa_dir (and then
recursively the write lock, but that's an explicitly recursive lock,
so that part should be ok).

Now, I don't think this can *really* deadlock with the normal order of
operations, because during mounting there is no other process that can
take those in the reverse order (since the filesystem isn't live), but
I do wonder if we should just release the reiserfs write lock over the
iputs. We release it in other parts anyway (like for the quota off)

Jeff, you already touched this exact case in commit d2d0395fd177
("reiserfs: locking, release lock around quota operations") except
that was for those quota operation cases.

Even if it's not a real problem, making lockdep happy sounds like a
good idea. Of course, the trouble is that this code path almost never
gets exercised (which is why this hasn't been noticed earlier), so
testing...

Jeff? Comments?

             Linus
--
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/