linux-kernel - Re: [GIT PULL] aio: a couple of fixes for 4.4

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:	Mon, 11 Jan 2016 17:59:42 +0100
From:	Jan Kara <jack@...e.cz>
To:	Benjamin LaHaise <bcrl@...ck.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	linux-aio@...ck.org, linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	Jan Kara <jack@...e.cz>, Dmitry Vyukov <dvyukov@...gle.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [GIT PULL] aio: a couple of fixes for 4.4

On Sat 09-01-16 18:00:36, Benjamin LaHaise wrote:
> On Sat, Jan 09, 2016 at 02:43:44PM -0800, Linus Torvalds wrote:
> > On Sat, Jan 9, 2016 at 2:08 PM, Benjamin LaHaise <bcrl@...ck.org> wrote:
> > >
> > > Please consider pulling the following changes to fix a couple of issues
> > > reported by Dmitry from git://git.kvack.org/~bcrl/aio-fixes.git .
> > 
> > No. This is much too late for this kind of hackery. That second patch
> > in particular is both subtle and ugly, and is messing with lockdep.
> 
> I wasn't particularly fond of how Jan implemented that.  Any ideas on a 
> better way to avoid lockdep complaining about this?  My initial feedback 
> to Jan was exactly that this should be handled within the file_start_write() 
> and file_end_write() APIs -- AIO really shouldn't need to be mucking in what 
> ought to be hidden behind that API.

[To introduce Peter to what we are talking about: This is about filesystem
freezing protection implemented effectively as a RW semaphore which must be
held for reading whenever we modify filesystem and held for writing when we
want to freeze the filesystem. With AIO, we need to return to userspace
with the lock held and thus lockdep complains.]

Thinking about this some more maybe we could improve lockdep in the following
way:

We could flag a locking class that it is OK to return with this lock to
userspace. When returning to userspace with such lock, lockdep would just
silently discard the information about lock being held. When freeing such
lock without lockdep knowing we acquired it (i.e., we were called by
userspace / IRQ when holding the lock), lockdep would just silently ignore
such request.

This way we could avoid the hacks with telling lockdep we dropped the lock
when we actually did not and we would also get improved coverage to the
current state. Peter, would something like the above be doable?

Bonus would be if we verified that all locks held when releasing lock we
"inherited" rank below the lock we are releasing (that way we would
simulate acquiring the lock when entering kernel from userspace). However
we would have to be careful about handling locks acquired via trylock
(those cannot create deadlocks) or when release happens from softirq /
irq - there we care only about locks acquired since softirq started - this
happens e.g. when we release the lock on IO completion like in the AIO
case.

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