diff mbox series

[v2] Bluetooth: Fix use-after-free bug caused by hci_cmd_timeout

Message ID 20240410035300.21160-1-duoming@zju.edu.cn
State New
Headers show
Series [v2] Bluetooth: Fix use-after-free bug caused by hci_cmd_timeout | expand

Commit Message

Duoming Zhou April 10, 2024, 3:53 a.m. UTC
When the kernel send a command to the hci device, the hdev->cmd_timer
will be scheduled to judge whether is command is timeout.

Meanwhile, the user space program uses ioctl(..., HCI_INQUIRY) to inquiry
the status of the command, it will wait the reply of the command until
the command is timeout. If it is timeout, the req_skb will be deallocated,
but req_skb could be dereferenced in hci_cmd_timeout() as well.

As a result, the use-after-free bug will happen. One of the scenarios
that could trigger the bug is shown below:

     thread 1                       thread2
hci_sock_ioctl()             | hci_sock_sendmsg()
 hci_inquiry()               |  queue_work(...,&hdev->cmd_work)
  hci_req_sync()             |   hci_cmd_work()
   __hci_req_sync()          |    queue_delayed_work(...,&hdev->cmd_timer)
    kfree_skb(hdev->req_skb) |     hci_cmd_timeout()
                             |      hci_skb_opcode(hdev->req_skb)

The KASan report for such a POC is shown below:

 ==================================================================
 BUG: KASAN: slab-use-after-free in hci_cmd_timeout+0x3c/0x110
 Read of size 2 at addr ffff8880087a1c78 by task kworker/u21:0/131
 ...
 Workqueue: hci0 hci_cmd_timeout
 Call Trace:
  <TASK>
  dump_stack_lvl+0x84/0xc0
  print_address_description+0x78/0x440
  print_report+0x11b/0x220
  ? pfn_valid+0xe4/0x140
  ? __virt_addr_valid+0x7c/0x90
  ? hci_cmd_timeout+0x3c/0x110
  kasan_report+0xc7/0x100
  ? hci_cmd_timeout+0x3c/0x110
  hci_cmd_timeout+0x3c/0x110
  process_one_work+0x2df/0x610
  worker_thread+0x72f/0x870
  ? pr_cont_work+0x280/0x280
  kthread+0x18a/0x1b0
  ? kthread_blkcg+0x50/0x50
  ret_from_fork+0x34/0x50
  ? kthread_blkcg+0x50/0x50
  ret_from_fork_asm+0x11/0x20
  </TASK>

 Allocated by task 131 on cpu 1 at 45.611537s:
  kasan_save_track+0x32/0x90
  __kasan_slab_alloc+0x4b/0x60
  kmem_cache_alloc+0xcb/0x240
  skb_clone+0x117/0x170
  hci_cmd_work+0x19c/0x2a0
  process_one_work+0x2df/0x610
  worker_thread+0x72f/0x870
  kthread+0x18a/0x1b0
  ret_from_fork+0x34/0x50
  ret_from_fork_asm+0x11/0x20

 Freed by task 135 on cpu 2 at 47.671850s:
  kasan_save_track+0x32/0x90
  kasan_save_free_info+0x40/0x50
  poison_slab_object+0x118/0x180
  __kasan_slab_free+0x12/0x30
  kmem_cache_free+0x92/0x200
  __hci_req_sync+0x2e2/0x350
  hci_req_sync+0x73/0x90
  hci_inquiry+0x1c1/0x350
  sock_do_ioctl+0x50/0x1a0
  sock_ioctl+0x2ea/0x3b0
  __se_sys_ioctl+0x89/0xd0
  do_syscall_64+0xc4/0x1b0
  entry_SYSCALL_64_after_hwframe+0x62/0x6a
  ...

In order to mitigate the bug, add cancel_delayed_work_sync() in
__hci_req_sync() to cancel the cmd_timer before the req_skb is
deallocated when the command is timeout.

Fixes: 9afee94939e3 ("Bluetooth: Fix memory leak at end of hci requests")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
---
Changes in v2:
  - Call cancel_delayed_work_sync() only if the command is timeout.

 net/bluetooth/hci_request.c | 1 +
 1 file changed, 1 insertion(+)

Comments

bluez.test.bot@gmail.com April 10, 2024, 4:33 a.m. UTC | #1
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=843066

---Test result---

Test Summary:
CheckPatch                    PASS      0.65 seconds
GitLint                       PASS      0.31 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      30.68 seconds
CheckAllWarning               PASS      33.50 seconds
CheckSparse                   PASS      38.47 seconds
CheckSmatch                   FAIL      34.98 seconds
BuildKernel32                 PASS      29.46 seconds
TestRunnerSetup               PASS      528.64 seconds
TestRunner_l2cap-tester       PASS      18.47 seconds
TestRunner_iso-tester         FAIL      35.00 seconds
TestRunner_bnep-tester        PASS      4.72 seconds
TestRunner_mgmt-tester        PASS      107.82 seconds
TestRunner_rfcomm-tester      PASS      7.34 seconds
TestRunner_sco-tester         PASS      14.96 seconds
TestRunner_ioctl-tester       PASS      7.83 seconds
TestRunner_mesh-tester        PASS      5.83 seconds
TestRunner_smp-tester         PASS      6.81 seconds
TestRunner_userchan-tester    PASS      4.87 seconds
IncrementalBuild              PASS      28.58 seconds

Details
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 121, Passed: 120 (99.2%), Failed: 1, Not Run: 0

Failed Test Cases
ISO Connect Suspend - Success                        Failed       4.183 seconds


---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 00e02138003..0b002f01816 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -174,6 +174,7 @@  int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req,
 
 	default:
 		err = -ETIMEDOUT;
+		cancel_delayed_work_sync(&hdev->cmd_timer);
 		if (hci_status)
 			*hci_status = HCI_ERROR_UNSPECIFIED;
 		break;