trezorhal: error checks, fix potential bug in usb_hid_class_data_out

micropython/trezorhal/usb.c

@@ -10,6 +10,10 @@
 #define USB_MAX_CONFIG_DESC_SIZE     128
 #define USB_MAX_STR_DESC_SIZE        256
 
+#define USB_EP_DIR_OUT              0x00
+#define USB_EP_DIR_IN               0x80
+#define USB_EP_DIR_MSK              0x80
+
 extern PCD_HandleTypeDef pcd_fs_handle;
 
 static USBD_HandleTypeDef usb_dev_handle;

micropython/trezorhal/usb_hid-impl.h

@@ -9,19 +9,20 @@
 /* usb_hid_add adds and configures new USB HID interface according to
  * configuration options passed in `info`. */
 int usb_hid_add(const usb_hid_info_t *info) {
-    usb_hid_descriptor_block_t *d = usb_desc_alloc_iface(sizeof(*d));
+    if ((info->iface_num >= USBD_MAX_NUM_INTERFACES) || (info->iface_num < usb_config_desc->bNumInterfaces)) {
-
+        return -1; // Invalid interface number
-    if (!d) {
+    }
-        return 1; // Not enough space in the configuration descriptor
+    if (((info->ep_in & USB_EP_DIR_MSK) == 0) || ((info->ep_out & USB_EP_DIR_MSK) != 0)) {
+        return -2; // Invalid endpoints
+    }
+    if ((info->rx_buffer == NULL) || (info->report_desc == NULL)) {
+        return -3; // Invalid buffers
     }
 
-    if ((info->iface_num < usb_config_desc->bNumInterfaces) ||
+    usb_hid_descriptor_block_t *d = usb_desc_alloc_iface(sizeof(usb_hid_descriptor_block_t));
-        (info->iface_num >= USBD_MAX_NUM_INTERFACES) ||
-        ((info->ep_in & 0x80) == 0) ||
-        ((info->ep_out & 0x80) != 0) ||
-        (info->rx_buffer == NULL)) {
 
-        return 1; // Invalid configuration values
+    if (!d) {
+        return -4; // Not enough space in the configuration descriptor
     }
 
     // Interface descriptor
@@ -61,7 +62,7 @@ int usb_hid_add(const usb_hid_info_t *info) {
     d->ep_out.bInterval        = info->polling_interval;
 
     // Config descriptor
-    usb_desc_add_iface(sizeof(*d));
+    usb_desc_add_iface(sizeof(usb_hid_descriptor_block_t));
 
     // Interface state
     usb_iface_t *i = &usb_ifaces[info->iface_num];
@@ -78,17 +79,35 @@ int usb_hid_add(const usb_hid_info_t *info) {
 }
 
 int usb_hid_can_read(uint8_t iface_num) {
-    return ((iface_num < USBD_MAX_NUM_INTERFACES) &&
+    if (usb_dev_handle.dev_state != USBD_STATE_CONFIGURED) {
-            (usb_ifaces[iface_num].type == USB_IFACE_TYPE_HID) &&
+        return 0; // Device is not configured
-            (usb_ifaces[iface_num].hid.rx_buffer_len > 0) &&
+    }
-            (usb_dev_handle.dev_state == USBD_STATE_CONFIGURED));
+    if (iface_num >= USBD_MAX_NUM_INTERFACES) {
+        return 0; // Invalid interface number
+    }
+    if (usb_ifaces[iface_num].type != USB_IFACE_TYPE_HID) {
+        return 0; // Invalid interface type
+    }
+    if (usb_ifaces[iface_num].hid.rx_buffer_len == 0) {
+        return 0; // Nothing in the receiving buffer
+    }
+    return 1;
 }
 
 int usb_hid_can_write(uint8_t iface_num) {
-    return ((iface_num < USBD_MAX_NUM_INTERFACES) &&
+    if (usb_dev_handle.dev_state != USBD_STATE_CONFIGURED) {
-            (usb_ifaces[iface_num].type == USB_IFACE_TYPE_HID) &&
+        return 0; // Device is not configured
-            (usb_ifaces[iface_num].hid.in_idle) &&
+    }
-            (usb_dev_handle.dev_state == USBD_STATE_CONFIGURED));
+    if (iface_num >= USBD_MAX_NUM_INTERFACES) {
+        return 0; // Invalid interface number
+    }
+    if (usb_ifaces[iface_num].type != USB_IFACE_TYPE_HID) {
+        return 0; // Invalid interface type
+    }
+    if (usb_ifaces[iface_num].hid.in_idle == 0) {
+        return 0; // Last transmission is not over yet
+    }
+    return 1;
 }
 
 int usb_hid_read(uint8_t iface_num, uint8_t *buf, uint32_t len) {
@@ -236,7 +255,7 @@ static int usb_hid_class_setup(USBD_HandleTypeDef *dev, usb_hid_state_t *state,
 }
 
 static uint8_t usb_hid_class_data_in(USBD_HandleTypeDef *dev, usb_hid_state_t *state, uint8_t ep_num) {
-    if ((ep_num | 0x80) == state->ep_in) {
+    if ((ep_num | USB_EP_DIR_IN) == state->ep_in) {
         // Ensure that the FIFO is empty before a new transfer,
         // this condition could be caused by a new transfer
         // before the end of the previous transfer.
@@ -251,10 +270,10 @@ static uint8_t usb_hid_class_data_out(USBD_HandleTypeDef *dev, usb_hid_state_t *
         // enough for state->max_packet_len bytes.
         state->rx_buffer_len = USBD_LL_GetRxDataSize(dev, ep_num);
 
-        if (state->rx_buffer_len > 0) {
         // Prepare the OUT EP to receive next packet
         USBD_LL_PrepareReceive(dev, ep_num, state->rx_buffer, state->max_packet_len);
 
+        if (state->rx_buffer_len > 0) {
             // Block the OUT EP until we process received data
             usb_ep_set_nak(dev, ep_num);
         }