| Message ID | 20210815233747.6969-1-kiran.k@intel.com |
|---|---|
| State | Accepted |
| Commit | ecb71f2566673553bc067e5b0036756871d0b9d3 |
| Headers | show |
| Series | [v2] Bluetooth: Fix race condition in handling NOP command | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=531739 ---Test result--- Test Summary: CheckPatch PASS 0.54 seconds GitLint PASS 0.10 seconds BuildKernel PASS 565.46 seconds TestRunner: Setup PASS 372.55 seconds TestRunner: l2cap-tester PASS 2.69 seconds TestRunner: bnep-tester PASS 1.96 seconds TestRunner: mgmt-tester PASS 33.15 seconds TestRunner: rfcomm-tester PASS 2.19 seconds TestRunner: sco-tester PASS 2.21 seconds TestRunner: smp-tester FAIL 2.24 seconds TestRunner: userchan-tester PASS 2.13 seconds Details ############################## Test: CheckPatch - PASS - 0.54 seconds Run checkpatch.pl script with rule in .checkpatch.conf ############################## Test: GitLint - PASS - 0.10 seconds Run gitlint with rule in .gitlint ############################## Test: BuildKernel - PASS - 565.46 seconds Build Kernel with minimal configuration supports Bluetooth ############################## Test: TestRunner: Setup - PASS - 372.55 seconds Setup environment for running Test Runner ############################## Test: TestRunner: l2cap-tester - PASS - 2.69 seconds Run test-runner with l2cap-tester Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: bnep-tester - PASS - 1.96 seconds Run test-runner with bnep-tester Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: mgmt-tester - PASS - 33.15 seconds Run test-runner with mgmt-tester Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3 ############################## Test: TestRunner: rfcomm-tester - PASS - 2.19 seconds Run test-runner with rfcomm-tester Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: sco-tester - PASS - 2.21 seconds Run test-runner with sco-tester Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: smp-tester - FAIL - 2.24 seconds Run test-runner with smp-tester Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 Failed Test Cases SMP Client - SC Request 2 Failed 0.028 seconds ############################## Test: TestRunner: userchan-tester - PASS - 2.13 seconds Run test-runner with userchan-tester Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 --- Regards, Linux Bluetooth
Hi Kiran, > For NOP command, need to cancel work scheduled on cmd_timer, > on receiving command status or commmand complete event. > > Below use case might lead to race condition multiple when NOP > commands are queued sequentially: > > hci_cmd_work() { > if (atomic_read(&hdev->cmd_cnt) { > . > . > . > atomic_dec(&hdev->cmd_cnt); > hci_send_frame(hdev,...); > schedule_delayed_work(&hdev->cmd_timer,...); > } > } > > On receiving event for first NOP, the work scheduled on hdev->cmd_timer > is not cancelled and second NOP is dequeued and sent to controller. > > While waiting for an event for second NOP command, work scheduled on > cmd_timer for the first NOP can get scheduled, resulting in sending third > NOP command (sending back to back NOP commands). This might > cause issues at controller side (like memory overrun, controller going > unresponsive) resulting in hci tx timeouts, hardware errors etc. > > The fix to this issue is to cancel the delayed work scheduled on > cmd_timer on receiving command status or command complete event for > NOP command (this patch handles NOP command same as any other SIG > command). > > Signed-off-by: Kiran K <kiran.k@intel.com> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com> > Acked-by: Manish Mandlik <mmandlik@google.com> > --- > net/bluetooth/hci_event.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 38decf474f31..f07c9c3726fe 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3310,11 +3310,9 @@ static void hci_remote_features_evt(struct hci_dev *hdev, hci_dev_unlock(hdev); } -static inline void handle_cmd_cnt_and_timer(struct hci_dev *hdev, - u16 opcode, u8 ncmd) +static inline void handle_cmd_cnt_and_timer(struct hci_dev *hdev, u8 ncmd) { - if (opcode != HCI_OP_NOP) - cancel_delayed_work(&hdev->cmd_timer); + cancel_delayed_work(&hdev->cmd_timer); if (!test_bit(HCI_RESET, &hdev->flags)) { if (ncmd) { @@ -3689,7 +3687,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb, break; } - handle_cmd_cnt_and_timer(hdev, *opcode, ev->ncmd); + handle_cmd_cnt_and_timer(hdev, ev->ncmd); hci_req_cmd_complete(hdev, *opcode, *status, req_complete, req_complete_skb); @@ -3790,7 +3788,7 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb, break; } - handle_cmd_cnt_and_timer(hdev, *opcode, ev->ncmd); + handle_cmd_cnt_and_timer(hdev, ev->ncmd); /* Indicate request completion if the command failed. Also, if * we're not waiting for a special event and we get a success
For NOP command, need to cancel work scheduled on cmd_timer, on receiving command status or commmand complete event. Below use case might lead to race condition multiple when NOP commands are queued sequentially: hci_cmd_work() { if (atomic_read(&hdev->cmd_cnt) { . . . atomic_dec(&hdev->cmd_cnt); hci_send_frame(hdev,...); schedule_delayed_work(&hdev->cmd_timer,...); } } On receiving event for first NOP, the work scheduled on hdev->cmd_timer is not cancelled and second NOP is dequeued and sent to controller. While waiting for an event for second NOP command, work scheduled on cmd_timer for the first NOP can get scheduled, resulting in sending third NOP command (sending back to back NOP commands). This might cause issues at controller side (like memory overrun, controller going unresponsive) resulting in hci tx timeouts, hardware errors etc. The fix to this issue is to cancel the delayed work scheduled on cmd_timer on receiving command status or command complete event for NOP command (this patch handles NOP command same as any other SIG command). Signed-off-by: Kiran K <kiran.k@intel.com> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com> Acked-by: Manish Mandlik <mmandlik@google.com> --- net/bluetooth/hci_event.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)