linux-kernel - Re: [GIT PULL v2] Preparation for BKL'ed ioctl removal

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: <20100428132112.GC12017@nowhere>
Date:	Wed, 28 Apr 2010 15:21:15 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Arnd Bergmann <arnd@...db.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Al Viro <viro@...iv.linux.org.uk>,
	Jan Blunck <jblunck@...e.de>, John Kacur <jkacur@...hat.com>
Subject: Re: [GIT PULL v2] Preparation for BKL'ed ioctl removal

On Tue, Apr 27, 2010 at 11:25:30AM +0200, Ingo Molnar wrote:
> 
> * Arnd Bergmann <arnd@...db.de> wrote:
> 
> > On Monday 26 April 2010, Ingo Molnar wrote:
> > > This could be done all automated for a hundred old drivers if need to be. 
> > > There would be no bkl_ioctl's left.
> > 
> > I don't think it can be fully automated. [...]
> 
> Corner cases are not a problem as long as the risk of them going unnoticed is 
> lower than the risk of a manual conversion introducing bugs.
> 
> > [...] For the majority of the modules, your approach would work fine, but 
> > there are still the well-known pitfalls in corner cases:
> > 
> > - recursive uses in functions outside of ioctl (possibly none left
> >   after the TTY layer is done, but who knows)
> 
> Not a problem even if there's any such usage left: lockdep will sort those out 
> very quickly.
> 
> > - lock-order problems with other mutexes (see DRM)
> 
> This too will be mapped out very quickly via lockdep.

And the hung task detector too which is the last resort to detect
uncovered resource dependencies (was really useful for reiserfs).

But the problem is among those people who may use such ancient drivers,
I guess few of them will have those debug config enabled.

And because there are almost no testers of these drivers, nobody/few will ever
notice the problem.

> > - code that depends on autorelease to allow one ioctl while another
> >   is sleeping. (a small number of drivers)
> 
> This is a real issue, and in fact it's an unknown: there may be an unknown 
> number of random sleep points within BKL codepaths that is being relied on in 
> creative ways.
> 
> Note that by introducing a mutex we (in most cases) make the locking 
> _stricter_, so the biggest risk from that is a lockup - which will be 
> debuggable via lockdep.

So, as explained above, lockdep won't even help here.

I mean, for callsites that are obvious, say when it is clear that
the bkl is leaf lock or doesn't introduce uncovered resource dependencies due
to non-release on sleep, we should do such conversion. And I guess most
drivers that use the bkl follow this scheme.

But for the others (rares I think), the operation looks unsafe to me.
If we don't have the hardware to test the driver, then lockdep and hung
task detectors are going to be useless.

That said, once we reach that point with 4 users of bkl remaining, may
be that will be time to buy such hardware for a symbolic $1 in obscure
places and do the tests. Or just git-rm if we are too lazy.

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