[PATCH] hci: fix double free in hci_req_sync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The approach taken to address the 'CVE-2024-35978' introduced another
double-free vulnerability. commit 45d355a926ab
("Bluetooth: Fix memory leak in hci_req_sync_complete()")

'hdev->req_skb' double free scenario:

 cpu1                        cpu2
 ====                        ====
 sock_ioctl
 sock_do_ioctl
 hci_sock_ioctl
 hci_dev_cmd
 hci_req_sync
 __hci_req_sync              hci_rx_work
 kfree_skb(hdev->req_skb)    hci_event_packet
 (sleep)                     hci_req_sync_complete
  \__ Longer times,          kfree_skb(hdev->req_skb)
      reproduce well         hdev->req_skb = NULL

The longer cpu1 sleep in '__hci_req_sync', the more reproducible it is.
We've tested it by inserting the 'msleep()' function, and it's frequently
at 1000ms, and It has been consistently reproducible at 2000ms.

We confirmed the detection with various workloads that cause CPU1 to sleep.
The call trace below is one of the KASAN has seen.

Bluetooth: hci0: unexpected cc 0x0c38 length: 249 > 2
==================================================================
BUG: KASAN: slab-use-after-free in skb_release_data+0x7d8/0x8a0 home/paran/linux/net/core/skbuff.c:1119
Write of size 1 at addr ffff00000947d3fe by task syz-executor.2/2010

CPU: 1 PID: 2010 Comm: syz-executor.2 Not tainted
6.10.0-rc4-00217-g35bb670d65fc-dirty #22 Hardware name: linux,dummy-virt (DT)
Call trace:
 dump_backtrace+0x318/0x348 home/paran/linux/arch/arm64/kernel/stacktrace.c:317
 show_stack+0x4c/0x80 home/paran/linux/arch/arm64/kernel/stacktrace.c:324
 __dump_stack home/paran/linux/lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x214/0x328 home/paran/linux/lib/dump_stack.c:114
 print_address_description home/paran/linux/mm/kasan/report.c:377 [inline]
 print_report+0x2ac/0x948 home/paran/linux/mm/kasan/report.c:488
 kasan_report+0xc8/0x148 home/paran/linux/mm/kasan/report.c:601
 __asan_report_store1_noabort+0x44/0x60 home/paran/linux/mm/kasan/report_generic.c:383
 skb_release_data+0x7d8/0x8a0 home/paran/linux/net/core/skbuff.c:1119
 skb_release_all+0x80/0xe0 home/paran/linux/net/core/skbuff.c:1173
 __kfree_skb home/paran/linux/net/core/skbuff.c:1187 [inline]
 kfree_skb_reason+0x138/0x3a8 home/paran/linux/net/core/skbuff.c:1223
 __hci_req_sync+0x404/0x948 [bluetooth]
 hci_req_sync+0xc0/0x138 [bluetooth]
 hci_dev_cmd+0x33c/0xc18 [bluetooth]
 hci_sock_ioctl+0x800/0xb68 [bluetooth]
 sock_do_ioctl+0xfc/0x2e0 home/paran/linux/net/socket.c:1222
 sock_ioctl+0x62c/0xab0 home/paran/linux/net/socket.c:1341
 vfs_ioctl+0x90/0x140 home/paran/linux/fs/ioctl.c:51
 __do_sys_ioctl home/paran/linux/fs/ioctl.c:907 [inline]
 __se_sys_ioctl home/paran/linux/fs/ioctl.c:893 [inline]
 __arm64_sys_ioctl+0x218/0x268 home/paran/linux/fs/ioctl.c:893
 __invoke_syscall home/paran/linux/arch/arm64/kernel/syscall.c:34 [inline]
 invoke_syscall+0xdc/0x460 home/paran/linux/arch/arm64/kernel/syscall.c:48
 el0_svc_common.constprop.0+0x2d4/0x3e8 home/paran/linux/arch/arm64/kernel/syscall.c:133
 do_el0_svc+0x60/0x98 home/paran/linux/arch/arm64/kernel/syscall.c:152
 el0_svc+0xc4/0x240 home/paran/linux/arch/arm64/kernel/entry-common.c:712
 el0t_64_sync_handler+0x120/0x130 home/paran/linux/arch/arm64/kernel/entry-common.c:730
 el0t_64_sync+0x190/0x198 home/paran/linux/arch/arm64/kernel/entry.S:598

Allocated by task 577:
 kasan_save_stack+0x48/0x90 home/paran/linux/mm/kasan/common.c:47
 kasan_save_track+0x38/0x60 home/paran/linux/mm/kasan/common.c:68
 kasan_save_alloc_info+0x64/0xc0 home/paran/linux/mm/kasan/generic.c:565
 unpoison_slab_object home/paran/linux/mm/kasan/common.c:312 [inline]
 __kasan_slab_alloc+0x100/0x110 home/paran/linux/mm/kasan/common.c:338
 kasan_slab_alloc home/paran/linux/./include/linux/kasan.h:201 [inline]
 slab_post_alloc_hook home/paran/linux/mm/slub.c:3941 [inline]
 slab_alloc_node home/paran/linux/mm/slub.c:4001 [inline]
 kmem_cache_alloc_noprof+0x2e8/0x630 home/paran/linux/mm/slub.c:4008
 skb_clone+0x1a4/0x4d0 home/paran/linux/net/core/skbuff.c:2052
 hci_cmd_work+0x78c/0x868 [bluetooth]
 process_one_work home/paran/linux/kernel/workqueue.c:3231 [inline]
 process_scheduled_works+0x9fc/0x1d98 home/paran/linux/kernel/workqueue.c:3312
 worker_thread+0x57c/0xf98 home/paran/linux/kernel/workqueue.c:3393
 kthread+0x3c8/0x478 home/paran/linux/kernel/kthread.c:389
 ret_from_fork+0x10/0x20 home/paran/linux/arch/arm64/kernel/entry.S:860

Freed by task 577:
 kasan_save_stack+0x48/0x90 home/paran/linux/mm/kasan/common.c:47
 kasan_save_track+0x38/0x60 home/paran/linux/mm/kasan/common.c:68
 kasan_save_free_info+0x64/0xf0 home/paran/linux/mm/kasan/generic.c:579
 poison_slab_object+0x168/0x270 home/paran/linux/mm/kasan/common.c:240
 __kasan_slab_free+0x34/0xa0 home/paran/linux/mm/kasan/common.c:256
 kasan_slab_free home/paran/linux/./include/linux/kasan.h:184 [inline]
 slab_free_hook home/paran/linux/mm/slub.c:2196 [inline]
 slab_free home/paran/linux/mm/slub.c:4437 [inline]
 kmem_cache_free+0x20c/0x670 home/paran/linux/mm/slub.c:4512
 kfree_skbmem+0x2b0/0x390 home/paran/linux/net/core/skbuff.c:1131
 __kfree_skb home/paran/linux/net/core/skbuff.c:1188 [inline]
 kfree_skb_reason+0x14c/0x3a8 home/paran/linux/net/core/skbuff.c:1223
 hci_req_sync_complete+0x114/0x308 [bluetooth]
 hci_event_packet+0xa10/0x12a0 [bluetooth]
 hci_rx_work+0x4d8/0xa80 [bluetooth]
 process_one_work home/paran/linux/kernel/workqueue.c:3231 [inline]
 process_scheduled_works+0x9fc/0x1d98 home/paran/linux/kernel/workqueue.c:3312
 worker_thread+0x57c/0xf98 home/paran/linux/kernel/workqueue.c:3393
 kthread+0x3c8/0x478 home/paran/linux/kernel/kthread.c:389
 ret_from_fork+0x10/0x20 home/paran/linux/arch/arm64/kernel/entry.S:860

The buggy address belongs to the object at ffff00000947d380
 which belongs to the cache skbuff_head_cache of size 232
The buggy address is located 126 bytes inside of
 freed 232-byte region [ffff00000947d380, ffff00000947d468)

The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x4947c
head: order:1 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
memcg:ffff0000222a73b1
flags: 0x3fffe0000000040(head|node=0|zone=0|lastcpupid=0x1ffff)
page_type: 0xffffefff(slab)
raw: 03fffe0000000040 ffff0000133a41c0 fffffdffc0e3a090 fffffdffc1069290
raw: 0000000000000000 0000000000120012 00000001ffffefff ffff0000222a73b1
head: 03fffe0000000040 ffff0000133a41c0 fffffdffc0e3a090 fffffdffc1069290
head: 0000000000000000 0000000000120012 00000001ffffefff ffff0000222a73b1
head: 03fffe0000000001 fffffdffc0251f01 ffffffffffffffff 0000000000000000
head: 0000000000000002 0000000000000000 00000000ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff00000947d280: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
 ffff00000947d300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff00000947d380: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                                ^
 ffff00000947d400: fb fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc
 ffff00000947d480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

We considered using the "hci_req_sync_lock" mutex, but concluded it
wasn't a good solution due to the increased sleep intervals causing
this issue. Instead, we introduced a spinlock member on 'struct hci_dev'.

Since applying our patch, we have repeatedly run the same tests in 
the syzkaller without encountering any issues.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=89a32741f4217856066c198a4a7267bcdd1edd67
Fixes: 45d355a926ab ("Bluetooth: Fix memory leak in hci_req_sync_complete()")
Signed-off-by: Levi Yun <yeoreum.yun@xxxxxxx>
Signed-off-by: Yunseong Kim <yskelg@xxxxxxxxx>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_core.c         |  1 +
 net/bluetooth/hci_request.c      |  6 ++----
 net/bluetooth/hci_request.h      |  9 +++++++++
 net/bluetooth/hci_sync.c         | 13 +++----------
 5 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c43716edf205..8b95061f063b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -519,6 +519,7 @@ struct hci_dev {
 	struct sk_buff		*recv_event;
 
 	struct mutex		req_lock;
+	spinlock_t		req_skb_lock;
 	wait_queue_head_t	req_wait_q;
 	__u32			req_status;
 	__u32			req_result;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dd3b0f501018..138a6b19894d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2572,6 +2572,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
 
 	mutex_init(&hdev->lock);
 	mutex_init(&hdev->req_lock);
+	spin_lock_init(&hdev->req_skb_lock);
 
 	ida_init(&hdev->unset_handle_ida);
 
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index efea25eb56ce..6a109c1ad359 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -106,8 +106,7 @@ void hci_req_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
 		hdev->req_result = result;
 		hdev->req_status = HCI_REQ_DONE;
 		if (skb) {
-			kfree_skb(hdev->req_skb);
-			hdev->req_skb = skb_get(skb);
+			hci_req_skb_release_and_set(hdev, skb_get(skb));
 		}
 		wake_up_interruptible(&hdev->req_wait_q);
 	}
@@ -181,8 +180,7 @@ int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req,
 		break;
 	}
 
-	kfree_skb(hdev->req_skb);
-	hdev->req_skb = NULL;
+	hci_req_skb_release_and_set(hdev, NULL);
 	hdev->req_status = hdev->req_result = 0;
 
 	bt_dev_dbg(hdev, "end: err %d", err);
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index c91f2838f542..6526c78443bc 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -28,6 +28,15 @@
 
 #define hci_req_sync_lock(hdev)   mutex_lock(&hdev->req_lock)
 #define hci_req_sync_unlock(hdev) mutex_unlock(&hdev->req_lock)
+#define hci_req_skb_release_and_set(hdev, val)		\
+({							\
+	if (hdev->req_skb) {				\
+		spin_lock(&hdev->req_skb_lock);		\
+		kfree_skb(hdev->req_skb);		\
+		hdev->req_skb = val;			\
+		spin_unlock(&hdev->req_skb_lock);	\
+	}						\
+})
 
 struct hci_request {
 	struct hci_dev		*hdev;
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index a8a7d2b36870..25c8d858c82e 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -33,8 +33,7 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
 	hdev->req_status = HCI_REQ_DONE;
 
 	/* Free the request command so it is not used as response */
-	kfree_skb(hdev->req_skb);
-	hdev->req_skb = NULL;
+	hci_req_skb_release_and_set(hdev, NULL);
 
 	if (skb) {
 		struct sock *sk = hci_skb_sk(skb);
@@ -4935,10 +4934,7 @@ int hci_dev_open_sync(struct hci_dev *hdev)
 			hdev->sent_cmd = NULL;
 		}
 
-		if (hdev->req_skb) {
-			kfree_skb(hdev->req_skb);
-			hdev->req_skb = NULL;
-		}
+		hci_req_skb_release_and_set(hdev, NULL);
 
 		clear_bit(HCI_RUNNING, &hdev->flags);
 		hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
@@ -5100,10 +5096,7 @@ int hci_dev_close_sync(struct hci_dev *hdev)
 	}
 
 	/* Drop last request */
-	if (hdev->req_skb) {
-		kfree_skb(hdev->req_skb);
-		hdev->req_skb = NULL;
-	}
+	hci_req_skb_release_and_set(hdev, NULL);
 
 	clear_bit(HCI_RUNNING, &hdev->flags);
 	hci_sock_dev_event(hdev, HCI_DEV_CLOSE);
-- 
2.45.2





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux