From 55d0416641414e7b69122b49fa3f0633bc3242b0 Mon Sep 17 00:00:00 2001 From: Pavol Rusnak Date: Sat, 27 Jan 2018 17:27:57 +0100 Subject: [PATCH] trezorhal: make usb stack more robust --- embed/bootloader/main.c | 2 +- embed/trezorhal/usb.c | 13 +++++++++++-- embed/trezorhal/usb_hid-impl.h | 28 ++++++++++++++++++++-------- embed/trezorhal/usb_vcp-impl.h | 26 ++++++++++---------------- embed/trezorhal/usb_webusb-impl.h | 30 ++++++++++++++---------------- 5 files changed, 56 insertions(+), 43 deletions(-) diff --git a/embed/bootloader/main.c b/embed/bootloader/main.c index 5d828ab7cd..9ed6bc1808 100644 --- a/embed/bootloader/main.c +++ b/embed/bootloader/main.c @@ -35,7 +35,7 @@ static void usb_init_all(void) { .product_id = 0x53C0, .release_num = 0x0200, .manufacturer = "SatoshiLabs", - .product = "TREZOR Bootloader", + .product = "TREZOR", .serial_number = "", .configuration = "", .interface = "TREZOR Interface", diff --git a/embed/trezorhal/usb.c b/embed/trezorhal/usb.c index 1693023e84..fc2b3142af 100644 --- a/embed/trezorhal/usb.c +++ b/embed/trezorhal/usb.c @@ -300,6 +300,7 @@ static uint8_t usb_class_setup(USBD_HandleTypeDef *dev, USBD_SetupReqTypedef *re ((req->bmRequest & USB_REQ_TYPE_MASK) != USB_REQ_TYPE_VENDOR)) { return USBD_OK; } + if ((req->bmRequest & USB_REQ_TYPE_MASK) == USB_REQ_TYPE_VENDOR) { if ((req->bmRequest & USB_REQ_RECIPIENT_MASK) == USB_REQ_RECIPIENT_DEVICE) { if (req->bRequest == USB_WEBUSB_VENDOR_CODE) { @@ -311,11 +312,14 @@ static uint8_t usb_class_setup(USBD_HandleTypeDef *dev, USBD_SetupReqTypedef *re 't', 'r', 'e', 'z', 'o', 'r', '.', 'i', 'o', '/', 's', 't', 'a', 'r', 't', // char URL[] }; USBD_CtlSendData(dev, UNCONST(webusb_url), sizeof(webusb_url)); + return USBD_OK; } else { + USBD_CtlError(dev, req); return USBD_FAIL; } } #if USE_WINUSB + else if (req->bRequest == USB_WINUSB_VENDOR_CODE) { if (req->wIndex == USB_WINUSB_REQ_GET_COMPATIBLE_ID_FEATURE_DESCRIPTOR) { static const uint8_t winusb_wcid[] = { @@ -333,7 +337,9 @@ static uint8_t usb_class_setup(USBD_HandleTypeDef *dev, USBD_SetupReqTypedef *re 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // reserved }; USBD_CtlSendData(dev, UNCONST(winusb_wcid), sizeof(winusb_wcid)); + return USBD_OK; } else { + USBD_CtlError(dev, req); return USBD_FAIL; } } @@ -359,14 +365,16 @@ static uint8_t usb_class_setup(USBD_HandleTypeDef *dev, USBD_SetupReqTypedef *re '{', 0x00, 'c', 0x00, '6', 0x00, 'c', 0x00, '3', 0x00, '7', 0x00, '4', 0x00, 'a', 0x00, '6', 0x00, '-', 0x00, '2', 0x00, '2', 0x00, '8', 0x00, '5', 0x00, '-', 0x00, '4', 0x00, 'c', 0x00, 'b', 0x00, '8', 0x00, '-', 0x00, 'a', 0x00, 'b', 0x00, '4', 0x00, '3', 0x00, '-', 0x00, '1', 0x00, '7', 0x00, '6', 0x00, '4', 0x00, '7', 0x00, 'c', 0x00, 'e', 0x00, 'a', 0x00, '5', 0x00, '0', 0x00, '3', 0x00, 'd', 0x00, '}', 0x00, 0x00, 0x00, 0x00, 0x00, // propertyData }; USBD_CtlSendData(dev, UNCONST(winusb_guid), sizeof(winusb_guid)); + return USBD_OK; } else { + USBD_CtlError(dev, req); return USBD_FAIL; } } } #endif - } - if (req->wIndex >= USBD_MAX_NUM_INTERFACES) { + } else if (req->wIndex >= USBD_MAX_NUM_INTERFACES) { + USBD_CtlError(dev, req); return USBD_FAIL; } switch (usb_ifaces[req->wIndex].type) { @@ -377,6 +385,7 @@ static uint8_t usb_class_setup(USBD_HandleTypeDef *dev, USBD_SetupReqTypedef *re case USB_IFACE_TYPE_WEBUSB: return usb_webusb_class_setup(dev, &usb_ifaces[req->wIndex].webusb, req); default: + USBD_CtlError(dev, req); return USBD_FAIL; } } diff --git a/embed/trezorhal/usb_hid-impl.h b/embed/trezorhal/usb_hid-impl.h index 2384599685..d54cdfe9a9 100644 --- a/embed/trezorhal/usb_hid-impl.h +++ b/embed/trezorhal/usb_hid-impl.h @@ -242,6 +242,7 @@ static void usb_hid_class_deinit(USBD_HandleTypeDef *dev, usb_hid_state_t *state } static int usb_hid_class_setup(USBD_HandleTypeDef *dev, usb_hid_state_t *state, USBD_SetupReqTypedef *req) { + switch (req->bmRequest & USB_REQ_TYPE_MASK) { // Class request @@ -250,19 +251,21 @@ static int usb_hid_class_setup(USBD_HandleTypeDef *dev, usb_hid_state_t *state, case USB_HID_REQ_SET_PROTOCOL: state->protocol = req->wValue; - break; + USBD_CtlSendStatus(dev); + return USBD_OK; case USB_HID_REQ_GET_PROTOCOL: USBD_CtlSendData(dev, &state->protocol, sizeof(state->protocol)); - break; + return USBD_OK; case USB_HID_REQ_SET_IDLE: state->idle_rate = req->wValue >> 8; - break; + USBD_CtlSendStatus(dev); + return USBD_OK; case USB_HID_REQ_GET_IDLE: USBD_CtlSendData(dev, &state->idle_rate, sizeof(state->idle_rate)); - break; + return USBD_OK; default: USBD_CtlError(dev, req); @@ -276,24 +279,33 @@ static int usb_hid_class_setup(USBD_HandleTypeDef *dev, usb_hid_state_t *state, case USB_REQ_SET_INTERFACE: state->alt_setting = req->wValue; - break; + USBD_CtlSendStatus(dev); + return USBD_OK; case USB_REQ_GET_INTERFACE: USBD_CtlSendData(dev, &state->alt_setting, sizeof(state->alt_setting)); - break; + return USBD_OK; case USB_REQ_GET_DESCRIPTOR: switch (req->wValue >> 8) { case USB_DESC_TYPE_HID: USBD_CtlSendData(dev, UNCONST(&state->desc_block->hid), MIN(req->wLength, sizeof(state->desc_block->hid))); - break; + return USBD_OK; case USB_DESC_TYPE_REPORT: USBD_CtlSendData(dev, UNCONST(state->report_desc), MIN(req->wLength, state->report_desc_len)); - break; + return USBD_OK; + + default: + USBD_CtlError(dev, req); + return USBD_FAIL; } break; + + default: + USBD_CtlError(dev, req); + return USBD_FAIL; } break; } diff --git a/embed/trezorhal/usb_vcp-impl.h b/embed/trezorhal/usb_vcp-impl.h index 292803fbf3..4bc023b05f 100644 --- a/embed/trezorhal/usb_vcp-impl.h +++ b/embed/trezorhal/usb_vcp-impl.h @@ -356,22 +356,16 @@ static int usb_vcp_class_setup(USBD_HandleTypeDef *dev, usb_vcp_state_t *state, return USBD_OK; } - switch (req->bmRequest & USB_REQ_DIR_MASK) { - case USB_REQ_DIR_D2H: - switch (req->bRequest) { - case USB_CDC_GET_LINE_CODING: - USBD_CtlSendData(dev, UNCONST(&line_coding), MIN(req->wLength, sizeof(line_coding))); - break; - default: - USBD_CtlSendData(dev, cmd_buffer, MIN(req->wLength, sizeof(cmd_buffer))); - break; - } - break; - case USB_REQ_DIR_H2D: - if (req->wLength > 0) { - USBD_CtlPrepareRx(dev, cmd_buffer, MIN(req->wLength, sizeof(cmd_buffer))); - } - break; + if ((req->bmRequest & USB_REQ_DIR_MASK) == USB_REQ_DIR_D2H) { + if (req->bRequest == USB_CDC_GET_LINE_CODING) { + USBD_CtlSendData(dev, UNCONST(&line_coding), MIN(req->wLength, sizeof(line_coding))); + } else { + USBD_CtlSendData(dev, cmd_buffer, MIN(req->wLength, sizeof(cmd_buffer))); + } + } else { // USB_REQ_DIR_H2D + if (req->wLength > 0) { + USBD_CtlPrepareRx(dev, cmd_buffer, MIN(req->wLength, sizeof(cmd_buffer))); + } } return USBD_OK; diff --git a/embed/trezorhal/usb_webusb-impl.h b/embed/trezorhal/usb_webusb-impl.h index 5f77864628..e91ae137f6 100644 --- a/embed/trezorhal/usb_webusb-impl.h +++ b/embed/trezorhal/usb_webusb-impl.h @@ -217,24 +217,22 @@ static void usb_webusb_class_deinit(USBD_HandleTypeDef *dev, usb_webusb_state_t static int usb_webusb_class_setup(USBD_HandleTypeDef *dev, usb_webusb_state_t *state, USBD_SetupReqTypedef *req) { - switch (req->bmRequest & USB_REQ_TYPE_MASK) { - - // Interface & Endpoint request - case USB_REQ_TYPE_STANDARD: - switch (req->bRequest) { - - case USB_REQ_SET_INTERFACE: - state->alt_setting = req->wValue; - break; - - case USB_REQ_GET_INTERFACE: - USBD_CtlSendData(dev, &state->alt_setting, sizeof(state->alt_setting)); - break; - } - break; + if ((req->bmRequest & USB_REQ_TYPE_MASK) != USB_REQ_TYPE_STANDARD) { + return USBD_OK; } - return USBD_OK; + switch (req->bRequest) { + case USB_REQ_SET_INTERFACE: + state->alt_setting = req->wValue; + USBD_CtlSendStatus(dev); + return USBD_OK; + case USB_REQ_GET_INTERFACE: + USBD_CtlSendData(dev, &state->alt_setting, sizeof(state->alt_setting)); + return USBD_OK; + default: + USBD_CtlError(dev, req); + return USBD_FAIL; + } } static void usb_webusb_class_data_in(USBD_HandleTypeDef *dev, usb_webusb_state_t *state, uint8_t ep_num) {