linux-kernel - Re: [PATCHv2] serial: samsung: drop the spinlock around uart_write

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:	Sun, 21 Feb 2016 10:37:32 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Peter Hurley <peter@...leysoftware.com>,
	Anand Moon <linux.amoon@...il.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Robert Bałdyga <r.baldyga@...sung.com>
Subject: Re: [PATCHv2] serial: samsung: drop the spinlock around uart_write_wakeup

2016-02-20 4:30 GMT+09:00 Peter Hurley <peter@...leysoftware.com>:
> [ +cc Krzysztof Kozlowski ]
>
> On 02/18/2016 09:15 PM, Anand Moon wrote:
>> From: Anand Moon <linux.amoon@...il.com>
>>
>> drop the spin_unlock/lock around uart_write_wakeup to protect
>> write wakeup for uart port.
>
> What Krzysztof was saying wrt v1 of this patch is that the
> changelog should provide as much information as possible to
> the maintainer(s) and driver author(s), and that you should
> test that outcome.
>
> Here's what I would have written for a commit message:
>
>
>   Remove deadlock workaround for line disciplines that invoke
>   the tty driver's write() method directly from their write_wakeup()
>   method. As documented for the write_wakeup() line discipline method
>   in tty_ldisc.h, line disciplines must not attempt i/o directly
>   from write_wakeup() as this will deadlock. Reviews of in-tree line
>   disciplines confirm all defer i/o.
>
>   NB: This workaround was added in commit c15c3747ee32
>   ("serial: samsung: fix potential soft lockup during uart write")
>   which notes both slip and bluetooth hci attempt i/o directly from
>   write_wakeup(). These issues were fixed in commits 661f7fda21b1
>   ("slip: Fix deadlock in write_wakeup") and da64c27d3c93
>   ("bluetooth: hci_ldisc: fix deadlock condition"), respectively.

Thanks Peter for thorough analysis. It shouldn't be done by you but by
the patch submitter... and I have big worries that Anand did not
perform that analysis.

Anand, could you at least test that this lockup does not happen
anymore? You will need board with Bluetooth for that (and not USB
Bluetooth...). If you cannot test it, maybe guys from Polish R&D could
help you (Cc-ed), because they were working on DMA for serial used in
Bluetooth.

Best regards,
Krzysztof