mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work
@ 2025-12-24 23:54 Szymon Wilczek
  2025-12-25  2:09 ` Hillf Danton
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Szymon Wilczek @ 2025-12-24 23:54 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz
  Cc: linux-bluetooth, linux-kernel, syzbot+4d6b203d625d2f57d4ca,
	Szymon Wilczek

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work
  2025-12-24 23:54 [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work Szymon Wilczek
@ 2025-12-25  2:09 ` Hillf Danton
  2025-12-25  9:19 ` [PATCH v2] Bluetooth: vhci: Fix slab-use-after-free by cloning skb Szymon Wilczek
  2025-12-25 10:12 ` [PATCH v3] " Szymon Wilczek
  2 siblings, 0 replies; 4+ messages in thread
From: Hillf Danton @ 2025-12-25  2:09 UTC (permalink / raw)
  To: Szymon Wilczek
  Cc: Luiz Augusto von Dentz, linux-bluetooth, linux-kernel,
	syzbot+4d6b203d625d2f57d4ca

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] Bluetooth: vhci: Fix slab-use-after-free by cloning skb
  2025-12-24 23:54 [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work Szymon Wilczek
  2025-12-25  2:09 ` Hillf Danton
@ 2025-12-25  9:19 ` Szymon Wilczek
  2025-12-25 10:12 ` [PATCH v3] " Szymon Wilczek
  2 siblings, 0 replies; 4+ messages in thread
From: Szymon Wilczek @ 2025-12-25  9:19 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, linux-kernel, syzbot+4d6b203d625d2f57d4ca,
	Szymon Wilczek

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3] Bluetooth: vhci: Fix slab-use-after-free by cloning skb
  2025-12-24 23:54 [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work Szymon Wilczek
  2025-12-25  2:09 ` Hillf Danton
  2025-12-25  9:19 ` [PATCH v2] Bluetooth: vhci: Fix slab-use-after-free by cloning skb Szymon Wilczek
@ 2025-12-25 10:12 ` Szymon Wilczek
  2 siblings, 0 replies; 4+ messages in thread
From: Szymon Wilczek @ 2025-12-25 10:12 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, linux-kernel, syzbot+4d6b203d625d2f57d4ca,
	Szymon Wilczek

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-25 10:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-24 23:54 [PATCH] Bluetooth: hci_core: Fix slab-use-after-free in hci_cmd_work Szymon Wilczek
2025-12-25  2:09 ` Hillf Danton
2025-12-25  9:19 ` [PATCH v2] Bluetooth: vhci: Fix slab-use-after-free by cloning skb Szymon Wilczek
2025-12-25 10:12 ` [PATCH v3] " Szymon Wilczek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®