[PATCH v1 0/2] Fix for proto races in hci_serdev.

[PATCH v1 0/2] Fix for proto races in hci_serdev.

[Balakrishna Godavarthi]

In recent testing we found that while removing hci_uart, we have seen
execution of hci_uart_write_work() after calling vendor specific
proto close. As we are freeing the vendor specific Tx and Rx buffers
in vendor close, execution of functions i.e. Rx or Tx functions may cause
a crash.

we already have a commit for hci_ldisc.c "e508e6026b19" and "048e1bd3a27f"
to overcome the race condition.

Changes of v1:
 
  * clearing flag HCI_UART_PROTO_READY while mnodule deinit such that
    we will not have any tractions further on Tx or Rx.
  * added check of HCI_UART_PROTO_READY while dequeuing an packet.
  

Balakrishna Godavarthi (2):
  Bluetooth: hci_serdev: clear HCI_UART_PROTO_READY to avoid closing
    proto races
  Bluetooth: hci_serdev: Add protocol check in hci_uart_dequeue().

 drivers/bluetooth/hci_serdev.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 1/2] Bluetooth: hci_serdev: clear HCI_UART_PROTO_READY to avoid closing proto races

[Balakrishna Godavarthi]

Clearing HCI_UART_PROTO_READY will avoid usage of proto function pointers
before running the proto close function pointer. There is chance of kernel
crash, due to usage of non proto close function pointers after proto close.

Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
 drivers/bluetooth/hci_serdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index aa2543b3c286..46e20444ba19 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -368,6 +368,7 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 {
 	struct hci_dev *hdev = hu->hdev;
 
+	clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 	hci_unregister_dev(hdev);
 	hci_free_dev(hdev);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v1 2/2] Bluetooth: hci_serdev: Add protocol check in hci_uart_dequeue().

[Balakrishna Godavarthi]

This will help to check the status of protocol while dequeuing an
skb packet. In some instaces we will end up kernel crash,
where proto close is called and we trying to dequeue an packet.

[  500.142902] [<ffffff80080f9ce4>] do_raw_spin_lock+0x1c/0xe0
[  500.148643] [<ffffff80088f1c7c>] _raw_spin_lock_irqsave+0x38/0x48
[  500.154917] [<ffffff8008780ce8>] skb_dequeue+0x28/0x84
[  500.160209] [<ffffff8000ad6f48>] 0xffffff8000ad6f48
[  500.165230] [<ffffff8000ad6610>] 0xffffff8000ad6610
[  500.170257] [<ffffff80080c7ce8>] process_one_work+0x238/0x3e4
[  500.176174] [<ffffff80080c8330>] worker_thread+0x2bc/0x3d4
[  500.181821] [<ffffff80080cdabc>] kthread+0x138/0x140
[  500.186945] [<ffffff80080844e0>] ret_from_fork+0x10/0x18

Signed-off-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
 drivers/bluetooth/hci_serdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 46e20444ba19..a541e6ee085e 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -57,9 +57,10 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu)
 {
 	struct sk_buff *skb = hu->tx_skb;
 
-	if (!skb)
-		skb = hu->proto->dequeue(hu);
-	else
+	if (!skb) {
+		if (test_bit(HCI_UART_PROTO_READY, &hu->flags))
+			skb = hu->proto->dequeue(hu);
+	} else
 		hu->tx_skb = NULL;
 
 	return skb;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 0/2] Fix for proto races in hci_serdev.

[Marcel Holtmann]

Hi Balakrishna,

> In recent testing we found that while removing hci_uart, we have seen
> execution of hci_uart_write_work() after calling vendor specific
> proto close. As we are freeing the vendor specific Tx and Rx buffers
> in vendor close, execution of functions i.e. Rx or Tx functions may cause
> a crash.
> 
> we already have a commit for hci_ldisc.c "e508e6026b19" and "048e1bd3a27f"
> to overcome the race condition.
> 
> Changes of v1:
> 
>  * clearing flag HCI_UART_PROTO_READY while mnodule deinit such that
>    we will not have any tractions further on Tx or Rx.
>  * added check of HCI_UART_PROTO_READY while dequeuing an packet.
> 
> 
> Balakrishna Godavarthi (2):
>  Bluetooth: hci_serdev: clear HCI_UART_PROTO_READY to avoid closing
>    proto races
>  Bluetooth: hci_serdev: Add protocol check in hci_uart_dequeue().
> 
> drivers/bluetooth/hci_serdev.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel