linux-kernel - Re: Converting dev->mutex into dev->spinlock ?

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: <Y965qEg0Re2QoQ7Q@rowland.harvard.edu>
Date:   Sat, 4 Feb 2023 15:01:44 -0500
From:   Alan Stern <stern@...land.harvard.edu>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        USB list <linux-usb@...r.kernel.org>,
        Hillf Danton <hdanton@...a.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: Converting dev->mutex into dev->spinlock ?

On Sun, Feb 05, 2023 at 02:09:40AM +0900, Tetsuo Handa wrote:
> On 2023/02/05 1:27, Alan Stern wrote:
> > On Sun, Feb 05, 2023 at 01:12:12AM +0900, Tetsuo Handa wrote:
> >> On 2023/02/05 0:34, Alan Stern wrote:
> >> Lockdep validation on dev->mutex being disabled is really annoying, and
> >> I want to make lockdep validation on dev->mutex enabled; that is the
> >> "drivers/core: Remove lockdep_set_novalidate_class() usage" patch.
> > 
> >> Even if it is always safe to acquire a child device's lock while holding
> >> the parent's lock, disabling lockdep checks completely on device's lock is
> >> not safe.
> > 
> > I understand the problem you want to solve, and I understand that it
> > can be frustrating.  However, I do not believe you will be able to
> > solve this problem.
> 
> That is a declaration that driver developers are allowed to take it for granted
> that driver callback functions can behave as if dev->mutex is not held. 

No it isn't.  It is a declaration that driver developers must be extra 
careful because lockdep is unable to detect locking errors involving 
dev->mutex.

> Some developers test their changes with lockdep enabled, and believe that their
> changes are correct because lockdep did not complain.
> https://syzkaller.appspot.com/bug?extid=9ef743bba3a17c756174 is an example.

How do you know developers are making this mistake?  That example 
doesn't show anything of the sort; the commit which introduced the bug 
says nothing about lockdep.

> We should somehow update driver core code to make it possible to keep lockdep
> checks enabled on dev->mutex.

I'm sorry, but that simply is not feasible.  It doesn't matter how much 
you want to do it or feel it is needed; there is no reasonable way to do 
it.  To take just one example, what you are saying implies that when a 
driver is probed for a device, it would not be allowed to register a 
child device.  That's a ridiculous restriction.

(I might also mention that dev->mutex is used by drivers in places 
outside of the driver core.  So even if you did magically manage to fix 
the driver core, the problem would still remain.)

Alan Stern