From 96422f262ed8cdcd4792a239d3ad6938831f67db Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Tue, 17 Jun 2025 22:48:07 +0200 Subject: [PATCH] usb: device: fix Bluetooth buffer handling In both implementation, when comparing received data length take into account that the buffer obtained from bt_buf_get_tx() stores the type at the top. The buffer types are H:4 and in the TX path we need to check for BT_HCI_H4_* types not BT_BUF_*. In the legacy implementation, the hci_acl_pkt_len() function obtains the length from the USB transaction buffer, which does not contain a buffer type at the top. In the new implementation, partially revert the changes and restore hci_pkt_get_len(), this will be required for any further changes anyway. Fixes commit f85d63a6cc63 ("Bluetooth: Remove USB H4 mode support"). Signed-off-by: Johann Fischer --- subsys/usb/device/class/bluetooth.c | 12 +++--- subsys/usb/device_next/class/bt_hci.c | 56 ++++++++++++++++++++------- 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/subsys/usb/device/class/bluetooth.c b/subsys/usb/device/class/bluetooth.c index 33774eef90d..7edd4792749 100644 --- a/subsys/usb/device/class/bluetooth.c +++ b/subsys/usb/device/class/bluetooth.c @@ -154,20 +154,20 @@ static void hci_tx_thread(void *p1, void *p2, void *p3) type = net_buf_pull_u8(buf); switch (type) { - case BT_BUF_EVT: + case BT_HCI_H4_EVT: usb_transfer_sync( bluetooth_ep_data[HCI_INT_EP_IDX].ep_addr, buf->data, buf->len, USB_TRANS_WRITE | USB_TRANS_NO_ZLP); break; - case BT_BUF_ACL_IN: + case BT_HCI_H4_ACL: usb_transfer_sync( bluetooth_ep_data[HCI_IN_EP_IDX].ep_addr, buf->data, buf->len, USB_TRANS_WRITE); break; default: - LOG_ERR("Unknown type %u", type); + LOG_ERR("Unsupported type %u", type); break; } @@ -200,11 +200,11 @@ static uint16_t hci_acl_pkt_len(const uint8_t *data, size_t data_len) struct bt_hci_acl_hdr *acl_hdr; size_t hdr_len = sizeof(*acl_hdr); - if (data_len - 1 < hdr_len) { + if (data_len < hdr_len) { return 0; } - acl_hdr = (struct bt_hci_acl_hdr *)(data + 1); + acl_hdr = (struct bt_hci_acl_hdr *)data; return sys_le16_to_cpu(acl_hdr->len) + hdr_len; } @@ -250,7 +250,7 @@ static void acl_read_cb(uint8_t ep, int size, void *priv) LOG_DBG("len %u, chunk %u", buf->len, size); } - if (buf != NULL && pkt_len == buf->len) { + if (buf != NULL && pkt_len == buf->len - 1) { k_fifo_put(&rx_queue, buf); LOG_DBG("put"); buf = NULL; diff --git a/subsys/usb/device_next/class/bt_hci.c b/subsys/usb/device_next/class/bt_hci.c index 13665c78a1d..c78e64bbfe1 100644 --- a/subsys/usb/device_next/class/bt_hci.c +++ b/subsys/usb/device_next/class/bt_hci.c @@ -210,14 +210,14 @@ static void bt_hci_tx_thread(void *p1, void *p2, void *p3) type = net_buf_pull_u8(bt_buf); switch (type) { - case BT_BUF_EVT: + case BT_HCI_H4_EVT: ep = bt_hci_get_int_in(c_data); break; - case BT_BUF_ACL_IN: + case BT_HCI_H4_ACL: ep = bt_hci_get_bulk_in(c_data); break; default: - LOG_ERR("Unknown type %u", type); + LOG_ERR("Unsupported type %u", type); continue; } @@ -273,19 +273,43 @@ static int bt_hci_acl_out_start(struct usbd_class_data *const c_data) return ret; } -static uint16_t hci_acl_pkt_len(struct net_buf *const buf) +static uint16_t hci_pkt_get_len(const uint8_t h4_type, + const uint8_t *data, const size_t size) { - struct bt_hci_acl_hdr *acl_hdr; - size_t hdr_len; + size_t hdr_len = 0; + uint16_t len = 0; - hdr_len = sizeof(*acl_hdr); - if (buf->len - 1 < hdr_len) { + switch (h4_type) { + case BT_HCI_H4_CMD: { + struct bt_hci_cmd_hdr *cmd_hdr; + + hdr_len = sizeof(*cmd_hdr); + cmd_hdr = (struct bt_hci_cmd_hdr *)data; + len = cmd_hdr->param_len + hdr_len; + break; + } + case BT_HCI_H4_ACL: { + struct bt_hci_acl_hdr *acl_hdr; + + hdr_len = sizeof(*acl_hdr); + acl_hdr = (struct bt_hci_acl_hdr *)data; + len = sys_le16_to_cpu(acl_hdr->len) + hdr_len; + break; + } + case BT_HCI_H4_ISO: { + struct bt_hci_iso_hdr *iso_hdr; + + hdr_len = sizeof(*iso_hdr); + iso_hdr = (struct bt_hci_iso_hdr *)data; + len = bt_iso_hdr_len(sys_le16_to_cpu(iso_hdr->len)) + hdr_len; + break; + } + default: + LOG_ERR("Unknown H4 buffer type"); return 0; } - acl_hdr = (struct bt_hci_acl_hdr *)(buf->data + 1); - - return sys_le16_to_cpu(acl_hdr->len) + hdr_len; + return (size < hdr_len) ? 0 : len; } static int bt_hci_acl_out_cb(struct usbd_class_data *const c_data, @@ -305,7 +329,9 @@ static int bt_hci_acl_out_cb(struct usbd_class_data *const c_data, goto restart_out_transfer; } - hci_data->acl_len = hci_acl_pkt_len(hci_data->acl_buf); + hci_data->acl_len = hci_pkt_get_len(BT_HCI_H4_ACL, + buf->data, + buf->len); LOG_DBG("acl_len %u, chunk %u", hci_data->acl_len, buf->len); @@ -330,7 +356,11 @@ static int bt_hci_acl_out_cb(struct usbd_class_data *const c_data, LOG_INF("len %u, chunk %u", hci_data->acl_buf->len, buf->len); } - if (hci_data->acl_buf != NULL && hci_data->acl_len == hci_data->acl_buf->len) { + /* + * The buffer obtained from bt_buf_get_tx() stores the type at the top. + * Take this into account when comparing received data length. + */ + if (hci_data->acl_buf != NULL && hci_data->acl_len == hci_data->acl_buf->len - 1) { k_fifo_put(&bt_hci_rx_queue, hci_data->acl_buf); hci_data->acl_buf = NULL; hci_data->acl_len = 0;