mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: btusb: revert use of devm_kzalloc in btusb
@ 2025-12-10 16:02 Raphael Pinsonneault-Thibeault
  2025-12-15 19:44 ` patchwork-bot+bluetooth
  0 siblings, 1 reply; 2+ messages in thread
From: Raphael Pinsonneault-Thibeault @ 2025-12-10 16:02 UTC (permalink / raw)
  To: marcel, luiz.dentz
  Cc: oliver, linux-bluetooth, linux-kernel, Raphael Pinsonneault-Thibeault

This reverts commit 98921dbd00c4e ("Bluetooth: Use devm_kzalloc in
btusb.c file").

In btusb_probe(), we use devm_kzalloc() to allocate the btusb data. This
ties the lifetime of all the btusb data to the binding of a driver to
one interface, INTF. In a driver that binds to other interfaces, ISOC
and DIAG, this is an accident waiting to happen.

The issue is revealed in btusb_disconnect(), where calling
usb_driver_release_interface(&btusb_driver, data->intf) will have devm
free the data that is also being used by the other interfaces of the
driver that may not be released yet.

To fix this, revert the use of devm and go back to freeing memory
explicitly.

Signed-off-by: Raphael Pinsonneault-Thibeault <rpthibeault@gmail.com>
---
Syzbot reported this UAF already and my commit 23d22f2f7176
("Bluetooth: btusb: reorder cleanup in btusb_disconnect to avoid UAF")
provided a fix that rearranged the
usb_driver_release_interface() calls so INTF was released last because
I (wrongly) assumed using devm like this was intentional.

This patch is motivated by a discussion prompted by Oliver Neukum:
https://lore.kernel.org/linux-bluetooth/aee37797-a280-47ea-91ac-487ddc124ac7@neukum.org/

link to patch for 23d22f2f7176:
https://lore.kernel.org/linux-bluetooth/20251105192839.895418-3-rpthibeault@gmail.com/

 drivers/bluetooth/btusb.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8ed3883ab8ee..ded09e94d296 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4052,7 +4052,7 @@ static int btusb_probe(struct usb_interface *intf,
 			return -ENODEV;
 	}
 
-	data = devm_kzalloc(&intf->dev, sizeof(*data), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
@@ -4075,8 +4075,10 @@ static int btusb_probe(struct usb_interface *intf,
 		}
 	}
 
-	if (!data->intr_ep || !data->bulk_tx_ep || !data->bulk_rx_ep)
+	if (!data->intr_ep || !data->bulk_tx_ep || !data->bulk_rx_ep) {
+		kfree(data);
 		return -ENODEV;
+	}
 
 	if (id->driver_info & BTUSB_AMP) {
 		data->cmdreq_type = USB_TYPE_CLASS | 0x01;
@@ -4131,8 +4133,10 @@ static int btusb_probe(struct usb_interface *intf,
 	data->recv_acl = hci_recv_frame;
 
 	hdev = hci_alloc_dev_priv(priv_size);
-	if (!hdev)
+	if (!hdev) {
+		kfree(data);
 		return -ENOMEM;
+	}
 
 	hdev->bus = HCI_USB;
 	hci_set_drvdata(hdev, data);
@@ -4406,6 +4410,7 @@ static int btusb_probe(struct usb_interface *intf,
 	if (data->reset_gpio)
 		gpiod_put(data->reset_gpio);
 	hci_free_dev(hdev);
+	kfree(data);
 	return err;
 }
 
@@ -4454,6 +4459,7 @@ static void btusb_disconnect(struct usb_interface *intf)
 	}
 
 	hci_free_dev(hdev);
+	kfree(data);
 }
 
 #ifdef CONFIG_PM

base-commit: 9d588a1140b9ae211581a7a154d0b806d8cd8238
-- 
2.43.0


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

* Re: [PATCH] Bluetooth: btusb: revert use of devm_kzalloc in btusb
  2025-12-10 16:02 [PATCH] Bluetooth: btusb: revert use of devm_kzalloc in btusb Raphael Pinsonneault-Thibeault
@ 2025-12-15 19:44 ` patchwork-bot+bluetooth
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+bluetooth @ 2025-12-15 19:44 UTC (permalink / raw)
  To: Raphael Pinsonneault-Thibeault
  Cc: marcel, luiz.dentz, oliver, linux-bluetooth, linux-kernel

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 10 Dec 2025 11:02:28 -0500 you wrote:
> This reverts commit 98921dbd00c4e ("Bluetooth: Use devm_kzalloc in
> btusb.c file").
> 
> In btusb_probe(), we use devm_kzalloc() to allocate the btusb data. This
> ties the lifetime of all the btusb data to the binding of a driver to
> one interface, INTF. In a driver that binds to other interfaces, ISOC
> and DIAG, this is an accident waiting to happen.
> 
> [...]

Here is the summary with links:
  - Bluetooth: btusb: revert use of devm_kzalloc in btusb
    https://git.kernel.org/bluetooth/bluetooth-next/c/3a7cf69ce3cf

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2025-12-15 19:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-10 16:02 [PATCH] Bluetooth: btusb: revert use of devm_kzalloc in btusb Raphael Pinsonneault-Thibeault
2025-12-15 19:44 ` patchwork-bot+bluetooth

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®