linux-kernel - Re: [PATCH v2] Bluetooth: hci_ldisc: Allow sleeping while proto locks are held.

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]

Date:   Thu, 26 Oct 2017 02:47:49 -0700
From:   "Life is hard, and then you die" <ronald@...ovation.ch>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Marcel Holtmann <marcel@...tmann.org>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Dean Jenkins <Dean_Jenkins@...tor.com>,
        linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Bluetooth: hci_ldisc: Allow sleeping while proto
 locks are held.


On Thu, Oct 26, 2017 at 08:58:28AM +0200, Lukas Wunner wrote:
> On Wed, Oct 25, 2017 at 10:14:53PM -0700, =?UTF-8?q?Ronald=20Tschal=C3=A4r?= wrote:
> > Commit dec2c92880cc5435381d50e3045ef018a762a917 ("Bluetooth: hci_ldisc:
> > Use rwlocking to avoid closing proto races") introduced locks in
> > hci_ldisc that are held while calling the proto functions. These locks
> > are rwlock's, and hence do not allow sleeping while they are held.
> > However, the proto functions that hci_bcm registers use mutexes and
> > hence need to be able to sleep.
> [...]
> > We can't replace the mutex in hci_bcm, because there are other calls
> > there that might sleep. Therefore this replaces the rwlock's in
> > hci_ldisc with rw_semaphore's (which allow sleeping). This is a safer
> > approach anyway as it reduces the restrictions on the proto callbacks.
> > Also, because acquiring write-lock is very rare compared to acquiring
> > the read-lock, the percpu variant of rw_semaphore is used.
> 
> The percpu_rw_semaphore is unusual (if fine I guess), it's only used
> by cgroups, uprobes and ext4 so far and I was unaware of its existence.

I mainly took my cue from the documentation, which indicated this was
better in scenarios where writes are rare. But if there are concerns
about it's use, or there is some use-case where tty's are opened and
close a lot without doing much in between, then it can be trivially
replaced with the non-percpu variant.

> I don't have the hardware to test this but the rationale and patch itself
> LGTM, so:
> 
> Reviewed-by: Lukas Wunner <lukas@...ner.de>

Thanks!


  Cheers,

  Ronald