linux-kernel - Re: [PATCH 1/3, RFC] misc char dev BKL pushdown

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: <8bd0f97a0805210922n2dc871b6y67377944cc60b566@mail.gmail.com>
Date:	Wed, 21 May 2008 12:22:46 -0400
From:	"Mike Frysinger" <vapier.adi@...il.com>
To:	"Jonathan Corbet" <corbet@....net>
Cc:	"Arnd Bergmann" <arnd@...db.de>, "Wu, Bryan" <Bryan.Wu@...log.com>,
	"Linus Torvalds" <torvalds@...ux-foundation.org>,
	"Ingo Molnar" <mingo@...e.hu>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	"Alan Cox" <alan@...rguk.ukuu.org.uk>,
	"Alexander Viro" <viro@....linux.org.uk>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3, RFC] misc char dev BKL pushdown

On Tue, May 20, 2008 at 7:25 PM, Jonathan Corbet wrote:
> Mike Frysinger wrote:
>> please drop the coreb.c changes from your patch
>
> At a minimum, I would hope such a request would say something like "I've
> looked at the driver's locking and am convinced that the BKL is not
> needed."  Have you done that?  There is a certain leap of faith involved
> in removing that protection from a driver.
>
> I decided to take a quick look...
>
> - You use spin_lock_irq(&coreb_lock) in a number of places, but you do
>  not take the lock in the interrupt handler.  You also do not take the
>  lock in coreb_write() or coreb_read(), so those can race with the
>  interrupt handler, with ioctl(), and with each other.

the lock is to protect one thing: coreb_status.  we lock around any
access to it, so it not being grabbed in the irq handler or any other
function where coreb_status is not utilized is irrelevant.  that means
the BKL is not needed in the driver.

the rest of your comments are more or less on target, but again
irrelevant to the topic of the BKL.  i'll keep them in mind when i
rewrite the driver, thanks.
-mike
--
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/