[PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work

Szymon Wilczek posted 1 patch 1 month, 2 weeks ago
net/bluetooth/hci_core.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work
Posted by Szymon Wilczek 1 month, 2 weeks ago
syzbot reported a slab-use-after-free in hci_cmd_work.
The issue is that hci_send_cmd_sync() consumes the skb reference
(either by passing it to the driver which frees it, or by calling
kfree_skb() on error), but the skb might be accessed after the call
in certain configurations or due to race conditions with the freeing
process (e.g. vhci_read).

Explicitly hold a reference to the skb using skb_get() before calling
hci_send_cmd_sync() and release it with kfree_skb() afterwards. This
ensures the skb object remains valid throughout the function call,
regardless of how hci_send_cmd_sync() handles the original reference.

Reported-by: syzbot+4d6b203d625d2f57d4ca@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4d6b203d625d2f57d4ca
Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
---
 net/bluetooth/hci_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8ccec73dce45..af4ef31295c4 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -4149,7 +4149,10 @@ static void hci_cmd_work(struct work_struct *work)
 		if (!skb)
 			return;
 
+		skb_get(skb);
 		err = hci_send_cmd_sync(hdev, skb);
+		kfree_skb(skb);
+
 		if (err)
 			return;
 
-- 
2.52.0
Re: [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work
Posted by Hillf Danton 1 month, 2 weeks ago
On Thu, 25 Dec 2025 00:54:07 +0100 Szymon Wilczek wrote:
> syzbot reported a slab-use-after-free in hci_cmd_work.
> The issue is that hci_send_cmd_sync() consumes the skb reference
> (either by passing it to the driver which frees it, or by calling
> kfree_skb() on error), but the skb might be accessed after the call
> in certain configurations or due to race conditions with the freeing
> process (e.g. vhci_read).
> 
> Explicitly hold a reference to the skb using skb_get() before calling
> hci_send_cmd_sync() and release it with kfree_skb() afterwards. This
> ensures the skb object remains valid throughout the function call,
> regardless of how hci_send_cmd_sync() handles the original reference.
> 
> Reported-by: syzbot+4d6b203d625d2f57d4ca@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=4d6b203d625d2f57d4ca
> Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
> ---
>  net/bluetooth/hci_core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8ccec73dce45..af4ef31295c4 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -4149,7 +4149,10 @@ static void hci_cmd_work(struct work_struct *work)
>  		if (!skb)
>  			return;
>  
> +		skb_get(skb);
>  		err = hci_send_cmd_sync(hdev, skb);
> +		kfree_skb(skb);
> +
>  		if (err)
>  			return;
>  
> -- 
> 2.52.0
> 
Given skb_dequeue() in both vhci_read() and hci_cmd_work(), what is
missed is the root cause of the issue.
[PATCH v3] Bluetooth: vhci: Fix slab-use-after-free by cloning skb
Posted by Szymon Wilczek 1 month, 1 week ago
Hillf Danton pointed out that the root cause of the UAF issue is the
lack of isolation between hci_core and vhci driver consumers.

vhci_send_frame() modifies the skb (via skb_push) and queues the
original skb to the readq for userspace consumption. This means the
hci_core caller sees a modified skb (corrupted headers/data pointer)
if it retains any reference. Furthermore, if vhci_read() frees the
skb immediately, hci_core might hit a Use-After-Free.

Other drivers (like btusb) create a new URB and context, isolating
the skb lifetime.

Fix this by cloning the skb in vhci_send_frame() before queueing.
The clone is modified and queued. The original skb is freed using
dev_consume_skb_any() which is safe in atomic context, satisfying
the HCI driver contract to consume the skb while ensuring the queued
object is distinct from the one passed by hci_core.

Reported-by: syzbot+4d6b203d625d2f57d4ca@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4d6b203d625d2f57d4ca
Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
---
v3: Replaced kfree_skb() with dev_consume_skb_any() to fix sleeping
    in atomic context warning reported by CI.
v2: Moved fix to vhci driver, using skb_clone to isolate ownership.
---
 drivers/bluetooth/hci_vhci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 2fef08254d78..7c72c635965c 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -74,13 +74,20 @@ static int vhci_flush(struct hci_dev *hdev)
 static int vhci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct vhci_data *data = hci_get_drvdata(hdev);
+	struct sk_buff *nskb;
 
-	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+	nskb = skb_clone(skb, GFP_ATOMIC);
+	if (!nskb)
+		return -ENOMEM;
+
+	memcpy(skb_push(nskb, 1), &hci_skb_pkt_type(skb), 1);
 
-	skb_queue_tail(&data->readq, skb);
+	skb_queue_tail(&data->readq, nskb);
 
 	if (atomic_read(&data->initialized))
 		wake_up_interruptible(&data->read_wait);
+
+	dev_consume_skb_any(skb);
 	return 0;
 }
 
-- 
2.52.0
[PATCH v2] Bluetooth: vhci: Fix slab-use-after-free by cloning skb
Posted by Szymon Wilczek 1 month, 1 week ago
Hillf Danton pointed out that the root cause of the UAF issue is the
lack of isolation between hci_core and vhci driver consumers.

vhci_send_frame() modifies the skb (via skb_push) and queues the
original skb to the readq for userspace consumption. This means the
hci_core caller sees a modified skb (corrupted headers/data pointer)
if it retains any reference. Furthermore, if vhci_read() frees the
skb immediately, hci_core might hit a Use-After-Free.

Other drivers (like btusb) create a new URB and context, isolating
the skb lifetime.

Fix this by cloning the skb in vhci_send_frame() before queueing.
The clone is modified and queued. The original skb is freed
immediately, satisfying the HCI driver contract to consume the skb
while ensuring the queued object is distinct from the one passed by
hci_core.

Reported-by: syzbot+4d6b203d625d2f57d4ca@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4d6b203d625d2f57d4ca
Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
---
 drivers/bluetooth/hci_vhci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 2fef08254d78..f2901a4b5b3a 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -74,13 +74,20 @@ static int vhci_flush(struct hci_dev *hdev)
 static int vhci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct vhci_data *data = hci_get_drvdata(hdev);
+	struct sk_buff *nskb;
 
-	memcpy(skb_push(skb, 1), &hci_skb_pkt_type(skb), 1);
+	nskb = skb_clone(skb, GFP_ATOMIC);
+	if (!nskb)
+		return -ENOMEM;
+
+	memcpy(skb_push(nskb, 1), &hci_skb_pkt_type(skb), 1);
 
-	skb_queue_tail(&data->readq, skb);
+	skb_queue_tail(&data->readq, nskb);
 
 	if (atomic_read(&data->initialized))
 		wake_up_interruptible(&data->read_wait);
+
+	kfree_skb(skb);
 	return 0;
 }
 
-- 
2.52.0