lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250402162737.3271704-1-chharry@google.com>
Date: Thu,  3 Apr 2025 00:26:34 +0800
From: Hsin-chen Chuang <chharry@...gle.com>
To: luiz.dentz@...il.com
Cc: Hsin-chen Chuang <chharry@...omium.org>, chromeos-bluetooth-upstreaming@...omium.org, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Johan Hedberg <johan.hedberg@...il.com>, 
	Marcel Holtmann <marcel@...tmann.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Ying Hsu <yinghsu@...omium.org>, linux-bluetooth@...r.kernel.org, 
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: [PATCH] Bluetooth: Introduce HCI Driver Packet

From: Hsin-chen Chuang <chharry@...omium.org>

Although commit 75ddcd5ad40e ("Bluetooth: btusb: Configure altsetting
for HCI_USER_CHANNEL") has enabled the HCI_USER_CHANNEL user to send out
SCO data through USB Bluetooth chips, it's observed that with the patch
HFP is flaky on most of the existing USB Bluetooth controllers: Intel
chips sometimes send out no packet for Transparent codec; MTK chips may
generate SCO data with a wrong handle for CVSD codec; RTK could split
the data with a wrong packet size for Transparent codec; ... etc.

To address the issue above one needs to reset the altsetting back to
zero when there is no active SCO connection, which is the same as the
BlueZ behavior, and another benefit is the bus doesn't need to reserve
bandwidth when no SCO connection.

This patch introduces a fundamental solution that lets the user space
program to configure the altsetting freely:
- Define the new packet type HCI_DRV_PKT which is specifically used for
  communication between the user space program and the Bluetooth drviers
- Define the btusb driver command HCI_DRV_OP_SWITCH_ALT_SETTING which
  indicates the expected altsetting from the user space program
- btusb intercepts the command and adjusts the Isoc endpoint
  correspondingly

This patch is tested on ChromeOS devices. The USB Bluetooth models
(CVSD, TRANS alt3, and TRANS alt6) could pass the stress HFP test narrow
band speech and wide band speech.

Cc: chromeos-bluetooth-upstreaming@...omium.org
Fixes: b16b327edb4d ("Bluetooth: btusb: add sysfs attribute to control USB alt setting")
Signed-off-by: Hsin-chen Chuang <chharry@...omium.org>
---

 drivers/bluetooth/btusb.c       | 112 ++++++++++++++++++++++++++++++++
 drivers/bluetooth/hci_drv_pkt.h |  62 ++++++++++++++++++
 include/net/bluetooth/hci.h     |   1 +
 include/net/bluetooth/hci_mon.h |   2 +
 net/bluetooth/hci_core.c        |   2 +
 net/bluetooth/hci_sock.c        |  12 +++-
 6 files changed, 189 insertions(+), 2 deletions(-)
 create mode 100644 drivers/bluetooth/hci_drv_pkt.h

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 5012b5ff92c8..644a0f13f8ee 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -26,6 +26,7 @@
 #include "btbcm.h"
 #include "btrtl.h"
 #include "btmtk.h"
+#include "hci_drv_pkt.h"
 
 #define VERSION "0.8"
 
@@ -2151,6 +2152,111 @@ static int submit_or_queue_tx_urb(struct hci_dev *hdev, struct urb *urb)
 	return 0;
 }
 
+static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts);
+
+static int btusb_drv_process_cmd(struct hci_dev *hdev, struct sk_buff *cmd_skb)
+{
+	struct hci_drv_cmd_hdr *hdr;
+	u16 opcode, cmd_len;
+
+	hdr = skb_pull_data(cmd_skb, sizeof(*hdr));
+	if (!hdr)
+		return -EILSEQ;
+
+	opcode = le16_to_cpu(hdr->opcode);
+	cmd_len = le16_to_cpu(hdr->len);
+	if (cmd_len != cmd_skb->len)
+		return -EILSEQ;
+
+	switch (opcode) {
+	case HCI_DRV_OP_READ_SUPPORTED_DRIVER_COMMANDS: {
+		struct hci_drv_resp_read_supported_driver_commands *resp;
+		struct sk_buff *resp_skb;
+		struct btusb_data *data = hci_get_drvdata(hdev);
+		int ret;
+		u16 num_commands = 1; /* SUPPORTED_DRIVER_COMMANDS */
+
+		if (data->isoc)
+			num_commands++; /* SWITCH_ALT_SETTING */
+
+		resp_skb = hci_drv_skb_alloc(
+			opcode, sizeof(*resp) + num_commands * sizeof(__le16),
+			GFP_KERNEL);
+		if (!resp_skb)
+			return -ENOMEM;
+
+		resp = skb_put(resp_skb,
+			       sizeof(*resp) + num_commands * sizeof(__le16));
+		resp->status = HCI_DRV_STATUS_SUCCESS;
+		resp->num_commands = cpu_to_le16(num_commands);
+		resp->commands[0] =
+			cpu_to_le16(HCI_DRV_OP_READ_SUPPORTED_DRIVER_COMMANDS);
+
+		if (data->isoc)
+			resp->commands[1] =
+				cpu_to_le16(HCI_DRV_OP_SWITCH_ALT_SETTING);
+
+		ret = hci_recv_frame(hdev, resp_skb);
+		if (ret)
+			return ret;
+
+		kfree_skb(cmd_skb);
+		return 0;
+	}
+	case HCI_DRV_OP_SWITCH_ALT_SETTING: {
+		struct hci_drv_cmd_switch_alt_setting *cmd;
+		struct hci_drv_resp_status *resp;
+		struct sk_buff *resp_skb;
+		int ret;
+		u8 status;
+
+		resp_skb = hci_drv_skb_alloc(opcode, sizeof(*resp), GFP_KERNEL);
+		if (!resp_skb)
+			return -ENOMEM;
+
+		cmd = skb_pull_data(cmd_skb, sizeof(*cmd));
+		if (!cmd || cmd_skb->len || cmd->new_alt > 6) {
+			status = HCI_DRV_STATUS_INVALID_PARAMETERS;
+		} else {
+			ret = btusb_switch_alt_setting(hdev, cmd->new_alt);
+			if (ret)
+				status = HCI_DRV_STATUS_UNSPECIFIED_ERROR;
+			else
+				status = HCI_DRV_STATUS_SUCCESS;
+		}
+
+		resp = skb_put(resp_skb, sizeof(*resp));
+		resp->status = status;
+
+		ret = hci_recv_frame(hdev, resp_skb);
+		if (ret)
+			return ret;
+
+		kfree_skb(cmd_skb);
+		return 0;
+	}
+	default: {
+		struct hci_drv_resp_status *resp;
+		struct sk_buff *resp_skb;
+		int ret;
+
+		resp_skb = hci_drv_skb_alloc(opcode, sizeof(*resp), GFP_KERNEL);
+		if (!resp_skb)
+			return -ENOMEM;
+
+		resp = skb_put(resp_skb, sizeof(*resp));
+		resp->status = HCI_DRV_STATUS_UNKNOWN_COMMAND;
+
+		ret = hci_recv_frame(hdev, resp_skb);
+		if (ret)
+			return ret;
+
+		kfree_skb(cmd_skb);
+		return 0;
+	}
+	}
+}
+
 static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct urb *urb;
@@ -2192,6 +2298,9 @@ static int btusb_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 			return PTR_ERR(urb);
 
 		return submit_or_queue_tx_urb(hdev, urb);
+
+	case HCI_DRV_PKT:
+		return btusb_drv_process_cmd(hdev, skb);
 	}
 
 	return -EILSEQ;
@@ -2669,6 +2778,9 @@ static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
 			return PTR_ERR(urb);
 
 		return submit_or_queue_tx_urb(hdev, urb);
+
+	case HCI_DRV_PKT:
+		return btusb_drv_process_cmd(hdev, skb);
 	}
 
 	return -EILSEQ;
diff --git a/drivers/bluetooth/hci_drv_pkt.h b/drivers/bluetooth/hci_drv_pkt.h
new file mode 100644
index 000000000000..800e0090f816
--- /dev/null
+++ b/drivers/bluetooth/hci_drv_pkt.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2025 Google Corporation
+ */
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci.h>
+
+struct hci_drv_cmd_hdr {
+	__le16	opcode;
+	__le16	len;
+} __packed;
+
+struct hci_drv_resp_hdr {
+	__le16	opcode;
+	__le16	len;
+} __packed;
+
+struct hci_drv_resp_status {
+	__u8	status;
+} __packed;
+
+#define HCI_DRV_STATUS_SUCCESS			0x00
+#define HCI_DRV_STATUS_UNSPECIFIED_ERROR	0x01
+#define HCI_DRV_STATUS_UNKNOWN_COMMAND		0x02
+#define HCI_DRV_STATUS_INVALID_PARAMETERS	0x03
+
+/* Common commands that make sense on all drivers start from 0x0000. */
+
+#define HCI_DRV_OP_READ_SUPPORTED_DRIVER_COMMANDS	0x0000
+struct hci_drv_resp_read_supported_driver_commands {
+	__u8	status;
+	__le16	num_commands;
+	__le16	commands[];
+} __packed;
+
+/* btusb specific commands start from 0x1135.
+ * No particular reason - It's my lucky number.
+ */
+
+#define HCI_DRV_OP_SWITCH_ALT_SETTING	0x1135
+struct hci_drv_cmd_switch_alt_setting {
+	__u8	new_alt;
+} __packed;
+
+static inline struct sk_buff *hci_drv_skb_alloc(u16 opcode, u16 plen, gfp_t how)
+{
+	struct hci_drv_resp_hdr *hdr;
+	struct sk_buff *skb;
+
+	skb = bt_skb_alloc(sizeof(*hdr) + plen, how);
+	if (!skb)
+		return NULL;
+
+	hdr = skb_put(skb, sizeof(*hdr));
+	hdr->opcode = __cpu_to_le16(opcode);
+	hdr->len = __cpu_to_le16(plen);
+
+	hci_skb_pkt_type(skb) = HCI_DRV_PKT;
+
+	return skb;
+}
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index a8586c3058c7..e297b312d2b7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -494,6 +494,7 @@ enum {
 #define HCI_EVENT_PKT		0x04
 #define HCI_ISODATA_PKT		0x05
 #define HCI_DIAG_PKT		0xf0
+#define HCI_DRV_PKT		0xf1
 #define HCI_VENDOR_PKT		0xff
 
 /* HCI packet types */
diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
index 082f89531b88..bbd752494ef9 100644
--- a/include/net/bluetooth/hci_mon.h
+++ b/include/net/bluetooth/hci_mon.h
@@ -51,6 +51,8 @@ struct hci_mon_hdr {
 #define HCI_MON_CTRL_EVENT	17
 #define HCI_MON_ISO_TX_PKT	18
 #define HCI_MON_ISO_RX_PKT	19
+#define HCI_MON_DRV_TX_PKT	20
+#define HCI_MON_DRV_RX_PKT	21
 
 struct hci_mon_new_index {
 	__u8		type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5eb0600bbd03..bb4e1721edc2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2911,6 +2911,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
 		break;
 	case HCI_ISODATA_PKT:
 		break;
+	case HCI_DRV_PKT:
+		break;
 	default:
 		kfree_skb(skb);
 		return -EINVAL;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 022b86797acd..428ee5c7de7e 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -234,7 +234,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 			if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
 			    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 			    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT)
+			    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+			    hci_skb_pkt_type(skb) != HCI_DRV_PKT)
 				continue;
 		} else {
 			/* Don't send frame to other channel types */
@@ -391,6 +392,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
 		else
 			opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
 		break;
+	case HCI_DRV_PKT:
+		if (bt_cb(skb)->incoming)
+			opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
+		else
+			opcode = cpu_to_le16(HCI_MON_DRV_TX_PKT);
+		break;
 	case HCI_DIAG_PKT:
 		opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
 		break;
@@ -1860,7 +1867,8 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 		if (hci_skb_pkt_type(skb) != HCI_COMMAND_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
-		    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT) {
+		    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+		    hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
 			err = -EINVAL;
 			goto drop;
 		}
-- 
2.49.0.504.g3bcea36a83-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ