From 40b8239921509c1484a5ff812d6a6d5790e42006 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Tue, 10 Jun 2025 11:41:11 +0200 Subject: [PATCH 1/4] [nrf fromtree] usb: device_next: fix the null pointer dereference on FS devices With the commit fe3c001eeb8c ("usb: device_next: disable high-speed USB device descriptor if not used") there is no high-speed device descriptor by default. Signed-off-by: Johann Fischer (cherry picked from commit 569f4d6d182e9802b3db9fc4ae58ec33f64e10e1) --- subsys/usb/device_next/usbd_ch9.c | 17 +++++++++++------ subsys/usb/device_next/usbd_device.c | 10 ++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/subsys/usb/device_next/usbd_ch9.c b/subsys/usb/device_next/usbd_ch9.c index c9f806960d7..3adb3574f0c 100644 --- a/subsys/usb/device_next/usbd_ch9.c +++ b/subsys/usb/device_next/usbd_ch9.c @@ -683,12 +683,6 @@ static int sreq_get_dev_qualifier(struct usbd_context *const uds_ctx, struct usb_device_qualifier_descriptor q_desc = { .bLength = sizeof(struct usb_device_qualifier_descriptor), .bDescriptorType = USB_DESC_DEVICE_QUALIFIER, - .bcdUSB = d_desc->bcdUSB, - .bDeviceClass = d_desc->bDeviceClass, - .bDeviceSubClass = d_desc->bDeviceSubClass, - .bDeviceProtocol = d_desc->bDeviceProtocol, - .bMaxPacketSize0 = d_desc->bMaxPacketSize0, - .bNumConfigurations = d_desc->bNumConfigurations, .bReserved = 0U, }; size_t len; @@ -703,6 +697,17 @@ static int sreq_get_dev_qualifier(struct usbd_context *const uds_ctx, return 0; } + if (d_desc == NULL) { + return -EINVAL; + } + + q_desc.bcdUSB = d_desc->bcdUSB; + q_desc.bDeviceClass = d_desc->bDeviceClass; + q_desc.bDeviceSubClass = d_desc->bDeviceSubClass; + q_desc.bDeviceProtocol = d_desc->bDeviceProtocol; + q_desc.bMaxPacketSize0 = d_desc->bMaxPacketSize0; + q_desc.bNumConfigurations = d_desc->bNumConfigurations; + LOG_DBG("Get Device Qualifier"); len = MIN(setup->wLength, net_buf_tailroom(buf)); net_buf_add_mem(buf, &q_desc, MIN(len, q_desc.bLength)); diff --git a/subsys/usb/device_next/usbd_device.c b/subsys/usb/device_next/usbd_device.c index bb011e0731c..fcb689a5c56 100644 --- a/subsys/usb/device_next/usbd_device.c +++ b/subsys/usb/device_next/usbd_device.c @@ -66,6 +66,11 @@ int usbd_device_set_bcd_usb(struct usbd_context *const uds_ctx, } desc = get_device_descriptor(uds_ctx, speed); + if (desc == NULL) { + ret = -EINVAL; + goto set_bcd_exit; + } + desc->bcdUSB = sys_cpu_to_le16(bcd); set_bcd_exit: @@ -167,6 +172,11 @@ int usbd_device_set_code_triple(struct usbd_context *const uds_ctx, } desc = get_device_descriptor(uds_ctx, speed); + if (desc == NULL) { + ret = -EINVAL; + goto set_code_triple_exit; + } + desc->bDeviceClass = base_class; desc->bDeviceSubClass = subclass; desc->bDeviceProtocol = protocol; From f447ac99fe11a35dabf7d4372fd03ee8bcecbf81 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Tue, 17 Jun 2025 22:48:07 +0200 Subject: [PATCH 2/4] [nrf fromtree] 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 (cherry picked from commit 96422f262ed8cdcd4792a239d3ad6938831f67db) --- 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) { - return 0; + 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; - acl_hdr = (struct bt_hci_acl_hdr *)(buf->data + 1); + 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; + } - 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; From 9dbac91b2f43da30ee7b4dfacde69ab1d760af60 Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Tue, 17 Jun 2025 23:54:16 +0200 Subject: [PATCH 3/4] [nrf fromtree] usb: device_next: bt_hci: do not take semaphore if transfer enqueue fail Do not take semaphore if transfer enqueue fails. Signed-off-by: Johann Fischer (cherry picked from commit d2e42c99ac8090502125365b7eca45d002af27da) --- subsys/usb/device_next/class/bt_hci.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/subsys/usb/device_next/class/bt_hci.c b/subsys/usb/device_next/class/bt_hci.c index c78e64bbfe1..7a2dc6da598 100644 --- a/subsys/usb/device_next/class/bt_hci.c +++ b/subsys/usb/device_next/class/bt_hci.c @@ -189,8 +189,12 @@ static void bt_hci_tx_sync_in(struct usbd_class_data *const c_data, } net_buf_add_mem(buf, bt_buf->data, bt_buf->len); - usbd_ep_enqueue(c_data, buf); - k_sem_take(&hci_data->sync_sem, K_FOREVER); + if (usbd_ep_enqueue(c_data, buf)) { + LOG_ERR("Failed to enqueue transfer"); + } else { + k_sem_take(&hci_data->sync_sem, K_FOREVER); + } + net_buf_unref(buf); } From 419f482962290e0afadddfad3457724a4d27049f Mon Sep 17 00:00:00 2001 From: Johann Fischer Date: Fri, 6 Jun 2025 23:23:07 +0200 Subject: [PATCH 4/4] [nrf fromtree] usb: device_next: allow to use label as interface string descriptor The intention was to use the "interface-name" string property in the interface string descriptor, but using the label property is acceptable again. Therefore, allow the use of the DT label property string in the interface string descriptor. Follow exactly the same approach as in the CDC ACM implementation introduced in the commit b0791400f61f ("usb: device_next: cdc_acm: allow setting the interface description"). Signed-off-by: Johann Fischer (cherry picked from commit 79a80730b272f37292ca6743f058d5af55bfcea3) --- dts/bindings/usb/zephyr,hid-device.yaml | 7 +++--- samples/subsys/usb/hid-keyboard/app.overlay | 2 +- .../usb/hid-keyboard/large_in_report.overlay | 2 +- .../subsys/usb/hid-mouse/usbd_next.overlay | 2 +- subsys/usb/device_next/class/usbd_hid.c | 23 +++++++++++++++++++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/dts/bindings/usb/zephyr,hid-device.yaml b/dts/bindings/usb/zephyr,hid-device.yaml index 2d20b450986..1b055cf18d2 100644 --- a/dts/bindings/usb/zephyr,hid-device.yaml +++ b/dts/bindings/usb/zephyr,hid-device.yaml @@ -8,11 +8,10 @@ compatible: "zephyr,hid-device" include: base.yaml properties: - interface-name: - type: string + label: description: | - HID device name. When this property is present, a USB device will use it - as the string descriptor of the interface. + The string defined by the label property is also used for the USB device + interface string descriptor. protocol-code: type: string diff --git a/samples/subsys/usb/hid-keyboard/app.overlay b/samples/subsys/usb/hid-keyboard/app.overlay index 0d3d2ee7bd8..0905e8850bf 100644 --- a/samples/subsys/usb/hid-keyboard/app.overlay +++ b/samples/subsys/usb/hid-keyboard/app.overlay @@ -7,7 +7,7 @@ / { hid_dev_0: hid_dev_0 { compatible = "zephyr,hid-device"; - interface-name = "HID0"; + label = "HID0"; protocol-code = "keyboard"; in-report-size = <64>; in-polling-period-us = <1000>; diff --git a/samples/subsys/usb/hid-keyboard/large_in_report.overlay b/samples/subsys/usb/hid-keyboard/large_in_report.overlay index 93b47691a77..548342f4b3e 100644 --- a/samples/subsys/usb/hid-keyboard/large_in_report.overlay +++ b/samples/subsys/usb/hid-keyboard/large_in_report.overlay @@ -7,7 +7,7 @@ / { hid_dev_0: hid_dev_0 { compatible = "zephyr,hid-device"; - interface-name = "HID0"; + label = "HID0"; in-report-size = <256>; in-polling-period-us = <1000>; }; diff --git a/samples/subsys/usb/hid-mouse/usbd_next.overlay b/samples/subsys/usb/hid-mouse/usbd_next.overlay index ce74aa51879..86365a94ae5 100644 --- a/samples/subsys/usb/hid-mouse/usbd_next.overlay +++ b/samples/subsys/usb/hid-mouse/usbd_next.overlay @@ -7,7 +7,7 @@ / { hid_dev_0: hid_dev_0 { compatible = "zephyr,hid-device"; - interface-name = "HID0"; + label = "HID0"; protocol-code = "none"; in-polling-period-us = <1000>; in-report-size = <64>; diff --git a/subsys/usb/device_next/class/usbd_hid.c b/subsys/usb/device_next/class/usbd_hid.c index fd10eba3c85..d5476a88ed6 100644 --- a/subsys/usb/device_next/class/usbd_hid.c +++ b/subsys/usb/device_next/class/usbd_hid.c @@ -65,6 +65,7 @@ struct hid_device_config { struct usbd_class_data *c_data; struct net_buf_pool *pool_out; struct net_buf_pool *pool_in; + struct usbd_desc_node *const if_desc_data; const struct usb_desc_header **fs_desc; const struct usb_desc_header **hs_desc; }; @@ -488,8 +489,21 @@ static void *usbd_hid_get_desc(struct usbd_class_data *const c_data, static int usbd_hid_init(struct usbd_class_data *const c_data) { + struct usbd_context *uds_ctx = usbd_class_get_ctx(c_data); + const struct device *dev = usbd_class_get_private(c_data); + const struct hid_device_config *dcfg = dev->config; + struct usbd_hid_descriptor *const desc = dcfg->desc; + LOG_DBG("HID class %s init", c_data->name); + if (dcfg->if_desc_data != NULL && desc->if0.iInterface == 0) { + if (usbd_add_descriptor(uds_ctx, dcfg->if_desc_data)) { + LOG_ERR("Failed to add interface string descriptor"); + } else { + desc->if0.iInterface = usbd_str_desc_get_idx(dcfg->if_desc_data); + } + } + return 0; } @@ -750,6 +764,12 @@ static const struct hid_device_driver_api hid_device_api = { HID_OUT_POOL_DEFINE(n); \ USBD_HID_INTERFACE_DEFINE(n); \ \ + IF_ENABLED(DT_INST_NODE_HAS_PROP(n, label), ( \ + USBD_DESC_STRING_DEFINE(hid_if_desc_data_##n, \ + DT_INST_PROP(n, label), \ + USBD_DUT_STRING_INTERFACE); \ + )) \ + \ USBD_DEFINE_CLASS(hid_##n, \ &usbd_hid_api, \ (void *)DEVICE_DT_GET(DT_DRV_INST(n)), NULL); \ @@ -761,6 +781,9 @@ static const struct hid_device_driver_api hid_device_api = { .pool_out = HID_OUT_POOL_ADDR(n), \ .fs_desc = hid_fs_desc_##n, \ .hs_desc = hid_hs_desc_##n, \ + IF_ENABLED(DT_INST_NODE_HAS_PROP(n, label), ( \ + .if_desc_data = &hid_if_desc_data_##n, \ + )) \ }; \ \ static struct hid_device_data hid_data_##n; \