Bluetooth: use hdev->workqueue when queuing hdev->{cmd,ncmd}_timer works

syzbot is reporting attempt to schedule hdev->cmd_work work from system_wq
WQ into hdev->workqueue WQ which is under draining operation [1], for
commit c8efcc2589464ac7 ("workqueue: allow chained queueing during
destruction") does not allow such operation.

The check introduced by commit 877afadad2dce8aa ("Bluetooth: When HCI work
queue is drained, only queue chained work") was incomplete.

Use hdev->workqueue WQ when queuing hdev->{cmd,ncmd}_timer works because
hci_{cmd,ncmd}_timeout() calls queue_work(hdev->workqueue). Also, protect
the queuing operation with RCU read lock in order to avoid calling
queue_delayed_work() after cancel_delayed_work() completed.

Link: https://syzkaller.appspot.com/bug?extid=243b7d89777f90f7613b
Reported-by: syzbot <syzbot+243b7d89777f90f7613b@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: 877afadad2dce8aa ("Bluetooth: When HCI work queue is drained, only queue chained work")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3803e54f23c0f7aa8372451472cf7a2a45476365..66c7cdba0d323d4c7228ed3c3180fc5400eb1157 100644 (file)
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -597,6 +597,15 @@ static int hci_dev_do_reset(struct hci_dev *hdev)
 
        /* Cancel these to avoid queueing non-chained pending work */
        hci_dev_set_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE);
+       /* Wait for
+        *
+        *    if (!hci_dev_test_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE))
+        *        queue_delayed_work(&hdev->{cmd,ncmd}_timer)
+        *
+        * inside RCU section to see the flag or complete scheduling.
+        */
+       synchronize_rcu();
+       /* Explicitly cancel works in case scheduled after setting the flag. */
        cancel_delayed_work(&hdev->cmd_timer);
        cancel_delayed_work(&hdev->ncmd_timer);
 
@@ -4063,12 +4072,14 @@ static void hci_cmd_work(struct work_struct *work)
                        if (res < 0)
                                __hci_cmd_sync_cancel(hdev, -res);
 
+                       rcu_read_lock();
                        if (test_bit(HCI_RESET, &hdev->flags) ||
                            hci_dev_test_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE))
                                cancel_delayed_work(&hdev->cmd_timer);
                        else
-                               schedule_delayed_work(&hdev->cmd_timer,
-                                                     HCI_CMD_TIMEOUT);
+                               queue_delayed_work(hdev->workqueue, &hdev->cmd_timer,
+                                                  HCI_CMD_TIMEOUT);
+                       rcu_read_unlock();
                } else {
                        skb_queue_head(&hdev->cmd_q, skb);
                        queue_work(hdev->workqueue, &hdev->cmd_work);

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5acb6fa6d67658e8654354653f41c2e9b81c7b31..ed3e5b251af17d3349dec5e77f7fc5443a1ec61a 100644 (file)
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3767,16 +3767,18 @@ static inline void handle_cmd_cnt_and_timer(struct hci_dev *hdev, u8 ncmd)
 {
        cancel_delayed_work(&hdev->cmd_timer);
 
+       rcu_read_lock();
        if (!test_bit(HCI_RESET, &hdev->flags)) {
                if (ncmd) {
                        cancel_delayed_work(&hdev->ncmd_timer);
                        atomic_set(&hdev->cmd_cnt, 1);
                } else {
                        if (!hci_dev_test_flag(hdev, HCI_CMD_DRAIN_WORKQUEUE))
-                               schedule_delayed_work(&hdev->ncmd_timer,
-                                                     HCI_NCMD_TIMEOUT);
+                               queue_delayed_work(hdev->workqueue, &hdev->ncmd_timer,
+                                                  HCI_NCMD_TIMEOUT);
                }
        }
+       rcu_read_unlock();
 }
 
 static u8 hci_cc_le_read_buffer_size_v2(struct hci_dev *hdev, void *data,