* [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty.
@ 2017-10-26 5:15 =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
2017-10-26 6:52 ` Lukas Wunner
2017-10-30 14:49 ` Marcel Holtmann
0 siblings, 2 replies; 3+ messages in thread
From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= @ 2017-10-26 5:15 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: Dean Jenkins, Lukas Wunner, linux-bluetooth, linux-kernel
The following race condition still existed:
P1 P2
cancel_work_sync()
hci_uart_tx_wakeup()
hci_uart_write_work()
hci_uart_dequeue()
clear_bit(HCI_UART_PROTO_READY)
hci_unregister_dev(hdev)
hci_free_dev(hdev)
hu->proto->close(hu)
kfree(hu)
access to hdev and hu
Cancelling the work after clearing the HCI_UART_PROTO_READY bit avoids
this as any hci_uart_tx_wakeup() issued after the flag is cleared will
detect that and not schedule further work.
Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
Cc: Dean Jenkins <Dean_Jenkins@mentor.com>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
---
drivers/bluetooth/hci_ldisc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 31def781a562..c823914b3a80 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -523,13 +523,13 @@ static void hci_uart_tty_close(struct tty_struct *tty)
if (hdev)
hci_uart_close(hdev);
- cancel_work_sync(&hu->write_work);
-
if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
percpu_down_write(&hu->proto_lock);
clear_bit(HCI_UART_PROTO_READY, &hu->flags);
percpu_up_write(&hu->proto_lock);
+ cancel_work_sync(&hu->write_work);
+
if (hdev) {
if (test_bit(HCI_UART_REGISTERED, &hu->flags))
hci_unregister_dev(hdev);
--
2.13.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty.
2017-10-26 5:15 [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
@ 2017-10-26 6:52 ` Lukas Wunner
2017-10-30 14:49 ` Marcel Holtmann
1 sibling, 0 replies; 3+ messages in thread
From: Lukas Wunner @ 2017-10-26 6:52 UTC (permalink / raw)
To: =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, Dean Jenkins,
linux-bluetooth, linux-kernel
On Wed, Oct 25, 2017 at 10:15:19PM -0700, =?UTF-8?q?Ronald=20Tschal=C3=A4r?= wrote:
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -523,13 +523,13 @@ static void hci_uart_tty_close(struct tty_struct *tty)
> if (hdev)
> hci_uart_close(hdev);
>
> - cancel_work_sync(&hu->write_work);
> -
> if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
> percpu_down_write(&hu->proto_lock);
> clear_bit(HCI_UART_PROTO_READY, &hu->flags);
> percpu_up_write(&hu->proto_lock);
>
> + cancel_work_sync(&hu->write_work);
> +
Now the work is only cancelled if HCI_UART_PROTO_READY is set,
but that seems fine, so
Reviewed-by: Lukas Wunner <lukas@wunner.de>
It should be noted that this patch only applies cleanly if Ronald's
other patch is applied before.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty.
2017-10-26 5:15 [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
2017-10-26 6:52 ` Lukas Wunner
@ 2017-10-30 14:49 ` Marcel Holtmann
1 sibling, 0 replies; 3+ messages in thread
From: Marcel Holtmann @ 2017-10-30 14:49 UTC (permalink / raw)
To: =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
Cc: Gustavo F. Padovan, Johan Hedberg, Dean Jenkins, Lukas Wunner,
open list:BLUETOOTH DRIVERS, linux-kernel
Hi Ronald,
> The following race condition still existed:
>
> P1 P2
> cancel_work_sync()
> hci_uart_tx_wakeup()
> hci_uart_write_work()
> hci_uart_dequeue()
> clear_bit(HCI_UART_PROTO_READY)
> hci_unregister_dev(hdev)
> hci_free_dev(hdev)
> hu->proto->close(hu)
> kfree(hu)
> access to hdev and hu
>
> Cancelling the work after clearing the HCI_UART_PROTO_READY bit avoids
> this as any hci_uart_tx_wakeup() issued after the flag is cleared will
> detect that and not schedule further work.
>
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> Cc: Dean Jenkins <Dean_Jenkins@mentor.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Marcel Holtmann <marcel@holtmann.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Johan Hedberg <johan.hedberg@gmail.com>
> ---
> drivers/bluetooth/hci_ldisc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-30 14:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26 5:15 [PATCH] Bluetooth: hci_ldisc: Fix another race when closing the tty =?UTF-8?q?Ronald=20Tschal=C3=A4r?=
2017-10-26 6:52 ` Lukas Wunner
2017-10-30 14:49 ` Marcel Holtmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.