linux-kernel - Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks, which causes hangs during certain operations

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]

Message-ID: <alpine.LNX.2.00.1606011626060.11844@cbobk.fhfr.pm>
Date:	Wed, 1 Jun 2016 16:26:46 +0200 (CEST)
From:	Jiri Kosina <jikos@...nel.org>
To:	Roderick Colenbrander <roderick.colenbrander@...y.com>
cc:	dh.herrmann@...glemail.com, benjamin.tissoires@...hat.com,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH] HID: uhid: Fixes a bug with userspace bluetooth stacks,
 which causes hangs during certain operations

On Wed, 18 May 2016, Roderick Colenbrander wrote:

> Many devices use userspace bluetooth stacks like BlueZ or Bluedroid in combination
> with uhid. If any of these stacks is used with a HID device for which the driver
> performs a HID request as part .probe (or technically another HID operation),
> this results in a deadlock situation. The deadlock results in a 5 second timeout
> for I/O operations in HID drivers, so isn't fatal, but none of the I/O operations
> have a chance of succeeding.
> 
> The root cause for the problem is that uhid only allows for one request to be
> processed at a time per uhid instance and locks out other operations. This means
> that if a user space is creating a new HID device through 'UHID_CREATE', which
> ultimately triggers '.probe' through the HID layer. Then any HID request e.g. a
> read for calibration data would trigger a HID operation on uhid again, but it
> won't go out to userspace, because it is still stuck in UHID_CREATE.
> In addition bluetooth stacks are typically single threaded, so they wouldn't be
> able to handle any requests while waiting on uhid.
> 
> Lucikly the UHID spec is somewhat flexible and allows for fixing the issue,
> without breaking user space. The idea which the patch implements as discussed
> with David Herrmann is to decouple adding of a hid device (which triggers .probe)
> from UHID_CREATE. The work will kick off roughly once UHID_CREATE completed (or
> else will wait a tiny bit of time in .probe for a lock). A HID driver has to call
> HID to call 'hid_hw_start()' as part of .probe once it is ready for I/O, which
> triggers UHID_START to user space. Any HID operations should function now within
> .probe and won't deadlock because userspace is stuck on UHID_CREATE.
> 
> We verified this patch on Bluedroid with Android 6.0 and on desktop Linux with
> BlueZ stacks. Prior to the patch they had the deadlock issue.
> 
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@...y.com>

Thanks for the fix. I've applied it to 
hid.git#for-4.8/uhid-offload-hid-device-add

-- 
Jiri Kosina
SUSE Labs