fix(security): check usbh_control_transfer return value when bmRequestType is USB_REQUEST_DIR_IN

Signed-off-by: sakumisu <[email protected]>

18 files changed, 99 insertions, 70 deletions

diff --git a/class/aoa/usbh_aoa.c b/class/aoa/usbh_aoa.c
index 02d9fab5..22426dad 100644
--- a/class/aoa/usbh_aoa.c
+++ b/class/aoa/usbh_aoa.c
@@ -36,7 +36,7 @@ int usbh_aoa_switch(struct usbh_hubport *hport, struct aoa_string_info *info)
     setup->wLength = 2;
 
     ret = usbh_control_transfer(hport, setup, g_aoa_buffer);
-    if (ret < 0) {
+    if (ret < 2) {
         return ret;
     }
 
diff --git a/class/audio/usbh_audio.c b/class/audio/usbh_audio.c
index 185e4f3e..c401e83e 100644
--- a/class/audio/usbh_audio.c
+++ b/class/audio/usbh_audio.c
@@ -385,7 +385,7 @@ feature_found:
         setup->wLength = 2;
 
         ret = usbh_control_transfer(audio_class->hport, setup, g_audio_buf);
-        if (ret < 0) {
+        if (ret < 2) {
             return ret;
         }
 
@@ -400,7 +400,7 @@ feature_found:
         setup->wLength = 2;
 
         ret = usbh_control_transfer(audio_class->hport, setup, g_audio_buf);
-        if (ret < 0) {
+        if (ret < 2) {
             return ret;
         }
 
@@ -415,7 +415,7 @@ feature_found:
         setup->wLength = 2;
 
         ret = usbh_control_transfer(audio_class->hport, setup, g_audio_buf);
-        if (ret < 0) {
+        if (ret < 2) {
             return ret;
         }
         memcpy(volume_max, g_audio_buf, 2);
@@ -429,7 +429,7 @@ feature_found:
         setup->wLength = 2;
 
         ret = usbh_control_transfer(audio_class->hport, setup, g_audio_buf);
-        if (ret < 0) {
+        if (ret < 2) {
             return ret;
         }
         memcpy(volume_res, g_audio_buf, 2);
diff --git a/class/cdc/usbh_cdc_ncm.c b/class/cdc/usbh_cdc_ncm.c
index 225a7fcd..33cb7691 100644
--- a/class/cdc/usbh_cdc_ncm.c
+++ b/class/cdc/usbh_cdc_ncm.c
@@ -48,11 +48,11 @@ static int usbh_cdc_ncm_get_ntb_parameters(struct usbh_cdc_ncm *cdc_ncm_class, s
     setup->wLength = 28;
 
     ret = usbh_control_transfer(cdc_ncm_class->hport, setup, g_cdc_ncm_buf);
-    if (ret < 8) {
+    if (ret < sizeof(struct cdc_ncm_ntb_parameters)) {
         return ret;
     }
 
-    memcpy((uint8_t *)param, g_cdc_ncm_buf, MIN(ret - 8, sizeof(struct cdc_ncm_ntb_parameters)));
+    memcpy((uint8_t *)param, g_cdc_ncm_buf, sizeof(struct cdc_ncm_ntb_parameters));
     return 0;
 }
 
diff --git a/class/hid/usbh_hid.c b/class/hid/usbh_hid.c
index 3039f5aa..f8b89786 100644
--- a/class/hid/usbh_hid.c
+++ b/class/hid/usbh_hid.c
@@ -103,10 +103,10 @@ int usbh_hid_get_idle(struct usbh_hid *hid_class, uint8_t *buffer)
     setup->wLength = 1;
 
     ret = usbh_control_transfer(hid_class->hport, setup, g_hid_buf[hid_class->minor]);
-    if (ret < 8) {
+    if (ret < 1) {
         return ret;
     }
-    memcpy(buffer, g_hid_buf[hid_class->minor], MIN((uint32_t)ret - 8, 1));
+    *buffer = g_hid_buf[hid_class->minor][0];
     return ret;
 }
 
@@ -145,10 +145,10 @@ int usbh_hid_get_protocol(struct usbh_hid *hid_class, uint8_t *protocol)
     setup->wLength = 1;
 
     ret = usbh_control_transfer(hid_class->hport, setup, g_hid_buf[hid_class->minor]);
-    if (ret < 8) {
+    if (ret < 1) {
         return ret;
     }
-    memcpy(protocol, g_hid_buf[hid_class->minor], MIN((uint32_t)ret - 8, 1));
+    *protocol = g_hid_buf[hid_class->minor][0];
     return ret;
 }
 
@@ -324,7 +324,7 @@ int usbh_hid_parse_report_descriptor(const uint8_t *report_data, uint32_t report
         if (itemsize == 3) /* HID spec: 6.2.2.2 - Short Items */
             itemsize = 4;
 
-        if((itemsize + i + 1) > report_size) {
+        if ((itemsize + i + 1) > report_size) {
             goto err;
         }
 
@@ -450,7 +450,7 @@ int usbh_hid_report_convert(struct usbh_hid_report_item *item, const uint8_t *re
     const uint8_t *src;
     uint32_t bits_len = item->attribute.report_size * item->attribute.report_count;
 
-    if(bits_len == 0) {
+    if (bits_len == 0) {
         return -1;
     }
 
diff --git a/class/hub/usbh_hub.c b/class/hub/usbh_hub.c
index 26601d01..b53be6a3 100644
--- a/class/hub/usbh_hub.c
+++ b/class/hub/usbh_hub.c
@@ -71,7 +71,7 @@ static int _usbh_hub_get_hub_descriptor(struct usbh_hub *hub, uint8_t *buffer)
     setup->wLength = USB_SIZEOF_HUB_DESC;
 
     ret = usbh_control_transfer(hub->parent, setup, g_hub_buf[hub->bus->busid]);
-    if (ret < 0) {
+    if (ret < USB_SIZEOF_HUB_DESC) {
         return ret;
     }
     memcpy(buffer, g_hub_buf[hub->bus->busid], USB_SIZEOF_HUB_DESC);
@@ -93,7 +93,7 @@ static int _usbh_hub_get_hub_ss_descriptor(struct usbh_hub *hub, uint8_t *buffer
     setup->wLength = USB_SIZEOF_HUB_SS_DESC;
 
     ret = usbh_control_transfer(hub->parent, setup, g_hub_buf[hub->bus->busid]);
-    if (ret < 0) {
+    if (ret < USB_SIZEOF_HUB_SS_DESC) {
         return ret;
     }
     memcpy(buffer, g_hub_buf[hub->bus->busid], USB_SIZEOF_HUB_SS_DESC);
@@ -115,7 +115,7 @@ static int _usbh_hub_get_portstatus(struct usbh_hub *hub, uint8_t port, struct h
     setup->wLength = 4;
 
     ret = usbh_control_transfer(hub->parent, setup, g_hub_buf[hub->bus->busid]);
-    if (ret < 0) {
+    if (ret < 4) {
         return ret;
     }
     memcpy(port_status, g_hub_buf[hub->bus->busid], 4);
diff --git a/class/msc/usbh_msc.c b/class/msc/usbh_msc.c
index cec9eb0c..35db3447 100644
--- a/class/msc/usbh_msc.c
+++ b/class/msc/usbh_msc.c
@@ -52,6 +52,7 @@ static void usbh_msc_class_free(struct usbh_msc *msc_class)
 static int usbh_msc_get_maxlun(struct usbh_msc *msc_class, uint8_t *buffer)
 {
     struct usb_setup_packet *setup;
+    int ret;
 
     if (!msc_class || !msc_class->hport) {
         return -USB_ERR_INVAL;
@@ -64,7 +65,12 @@ static int usbh_msc_get_maxlun(struct usbh_msc *msc_class, uint8_t *buffer)
     setup->wIndex = msc_class->intf;
     setup->wLength = 1;
 
-    return usbh_control_transfer(msc_class->hport, setup, buffer);
+    ret = usbh_control_transfer(msc_class->hport, setup, buffer);
+    if (ret < 1) {
+        return ret;
+    }
+
+    return ret;
 }
 
 static void usbh_msc_cbw_dump(struct CBW *cbw)
diff --git a/class/serial/usbh_cdc_acm.c b/class/serial/usbh_cdc_acm.c
index cf5363ef..48643cb3 100644
--- a/class/serial/usbh_cdc_acm.c
+++ b/class/serial/usbh_cdc_acm.c
@@ -108,7 +108,7 @@ static int usbh_cdc_acm_get_line_coding(struct usbh_serial *serial, struct cdc_l
     setup->wLength = 7;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < sizeof(struct cdc_line_coding)) {
         return ret;
     }
     memcpy(line_coding, serial->iobuffer, sizeof(struct cdc_line_coding));
diff --git a/class/serial/usbh_ch34x.c b/class/serial/usbh_ch34x.c
index 4595bcae..a5fcd334 100644
--- a/class/serial/usbh_ch34x.c
+++ b/class/serial/usbh_ch34x.c
@@ -103,7 +103,7 @@ static int usbh_ch34x_control_in(struct usbh_serial *serial, uint8_t bRequest, u
     setup->wLength = size;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < size) {
         return ret;
     }
     memcpy(data, serial->iobuffer, size);
diff --git a/class/serial/usbh_cp210x.c b/class/serial/usbh_cp210x.c
index 7b240e2c..0f645a17 100644
--- a/class/serial/usbh_cp210x.c
+++ b/class/serial/usbh_cp210x.c
@@ -181,7 +181,7 @@ static int usbh_cp210x_control_in(struct usbh_serial *serial, uint8_t bRequest,
     setup->wLength = size;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < size) {
         return ret;
     }
     memcpy(data, serial->iobuffer, size);
diff --git a/class/serial/usbh_ftdi.c b/class/serial/usbh_ftdi.c
index c2d3a77b..19f00110 100644
--- a/class/serial/usbh_ftdi.c
+++ b/class/serial/usbh_ftdi.c
@@ -340,8 +340,8 @@ static int usbh_ftdi_get_modem_status(struct usbh_serial *serial)
     setup->wLength = 2;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
-        return 0;
+    if (ret < 2) {
+        return ret;
     }
 
     status = (serial->iobuffer[0] & FTDI_SIO_DSR_MASK ? USBH_SERIAL_TIOCM_DSR : 0) |
diff --git a/class/serial/usbh_pl2303.c b/class/serial/usbh_pl2303.c
index 062f8128..e3b6a307 100644
--- a/class/serial/usbh_pl2303.c
+++ b/class/serial/usbh_pl2303.c
@@ -242,10 +242,10 @@ static int pl2303_vendor_read(struct usbh_serial *serial, uint16_t wValue, uint8
     setup->wLength = 1;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < 1) {
         return ret;
     }
-    memcpy(data, serial->iobuffer, 1);
+    *data = serial->iobuffer[0];
 
     return ret;
 }
@@ -280,7 +280,7 @@ static bool pl2303_is_hxd_clone(struct usbh_serial *serial)
     setup->wLength = 7;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < 7) {
         return false;
     }
     return true;
@@ -566,7 +566,7 @@ static int usbh_pl2303_get_line_coding(struct usbh_serial *serial, struct cdc_li
     setup->wLength = 7;
 
     ret = usbh_control_transfer(serial->hport, setup, serial->iobuffer);
-    if (ret < 0) {
+    if (ret < sizeof(struct cdc_line_coding)) {
         return ret;
     }
     memcpy(line_coding, serial->iobuffer, sizeof(struct cdc_line_coding));
diff --git a/class/vendor/net/usbh_asix.c b/class/vendor/net/usbh_asix.c
index 06a0d0ba..3e286851 100644
--- a/class/vendor/net/usbh_asix.c
+++ b/class/vendor/net/usbh_asix.c
@@ -71,10 +71,10 @@ static int usbh_asix_read_cmd(struct usbh_asix *asix_class,
     setup->wLength = size;
 
     ret = usbh_control_transfer(asix_class->hport, setup, g_asix_buf);
-    if (ret < 8) {
+    if (ret < size) {
         return ret;
     }
-    memcpy(data, g_asix_buf, MIN(ret - 8, size));
+    memcpy(data, g_asix_buf, size);
 
     return ret;
 }
diff --git a/class/vendor/net/usbh_rtl8152.c b/class/vendor/net/usbh_rtl8152.c
index 50269f26..1cbf9b4f 100644
--- a/class/vendor/net/usbh_rtl8152.c
+++ b/class/vendor/net/usbh_rtl8152.c
@@ -962,10 +962,10 @@ static int usbh_rtl8152_read_regs(struct usbh_rtl8152 *rtl8152_class,
     setup->wLength = size;
 
     ret = usbh_control_transfer(rtl8152_class->hport, setup, g_rtl8152_buf);
-    if (ret < 8) {
+    if (ret < size) {
         return ret;
     }
-    memcpy(data, g_rtl8152_buf, MIN(ret - 8, size));
+    memcpy(data, g_rtl8152_buf, size);
 
     return ret;
 }
diff --git a/class/video/usbh_video.c b/class/video/usbh_video.c
index 8ecd252d..8e782e20 100644
--- a/class/video/usbh_video.c
+++ b/class/video/usbh_video.c
@@ -77,7 +77,7 @@ int usbh_video_get(struct usbh_video *video_class, uint8_t request, uint8_t intf
     retry = 0;
     while (1) {
         ret = usbh_control_transfer(video_class->hport, setup, g_video_buf);
-        if (ret > 0) {
+        if (ret >= len) {
             break;
         }
         retry++;
diff --git a/class/wireless/usbd_rndis.c b/class/wireless/usbd_rndis.c
index 5fceb98f..ea62c50e 100644
--- a/class/wireless/usbd_rndis.c
+++ b/class/wireless/usbd_rndis.c
@@ -28,7 +28,7 @@ struct usbd_rndis_priv {
     uint8_t mac[6];
 } g_usbd_rndis;
 
-#if CONFIG_USBDEV_RNDIS_RESP_BUFFER_SIZE < 140
+#if CONFIG_USBDEV_RNDIS_RESP_BUFFER_SIZE < 156
 #undef CONFIG_USBDEV_RNDIS_RESP_BUFFER_SIZE
 #define CONFIG_USBDEV_RNDIS_RESP_BUFFER_SIZE 156
 #endif
@@ -112,8 +112,7 @@ static int rndis_class_interface_request_handler(uint8_t busid, struct usb_setup
         case CDC_REQUEST_SEND_ENCAPSULATED_COMMAND:
             g_usbd_rndis.set_rsp_get = true;
 
-            rndis_encapsulated_cmd_handler(*data, setup->wLength);
-            break;
+            return rndis_encapsulated_cmd_handler(*data, setup->wLength);
         case CDC_REQUEST_GET_ENCAPSULATED_RESPONSE:
             g_usbd_rndis.set_rsp_get = false;
 
@@ -162,7 +161,9 @@ static int rndis_init_cmd_handler(uint8_t *data, uint32_t len)
     rndis_initialize_msg_t *cmd = (rndis_initialize_msg_t *)data;
     rndis_initialize_cmplt_t *resp;
 
-    (void)len;
+    if (len < sizeof(rndis_initialize_msg_t)) {
+        return -1;
+    }
 
     resp = ((rndis_initialize_cmplt_t *)rndis_encapsulated_resp_buffer);
     resp->RequestId = cmd->RequestId;
@@ -190,7 +191,10 @@ static int rndis_halt_cmd_handler(uint8_t *data, uint32_t len)
     rndis_halt_msg_t *resp;
 
     (void)data;
-    (void)len;
+
+    if (len < sizeof(rndis_halt_msg_t)) {
+        return -1;
+    }
 
     resp = ((rndis_halt_msg_t *)rndis_encapsulated_resp_buffer);
     resp->MessageLength = 0;
@@ -207,7 +211,9 @@ static int rndis_query_cmd_handler(uint8_t *data, uint32_t len)
     uint8_t *infomation_buffer;
     uint32_t infomation_len = 0;
 
-    (void)len;
+    if (len < sizeof(rndis_query_msg_t)) {
+        return -1;
+    }
 
     resp = ((rndis_query_cmplt_t *)rndis_encapsulated_resp_buffer);
     resp->MessageType = REMOTE_NDIS_QUERY_CMPLT;
@@ -354,8 +360,11 @@ static int rndis_set_cmd_handler(uint8_t *data, uint32_t len)
     rndis_set_msg_t *cmd = (rndis_set_msg_t *)data;
     rndis_set_cmplt_t *resp;
     rndis_config_parameter_t *param;
+    uint32_t *filter;
 
-    (void)len;
+    if (len < sizeof(rndis_set_msg_t)) {
+        return -1;
+    }
 
     resp = ((rndis_set_cmplt_t *)rndis_encapsulated_resp_buffer);
     resp->RequestId = cmd->RequestId;
@@ -367,21 +376,12 @@ static int rndis_set_cmd_handler(uint8_t *data, uint32_t len)
         case OID_GEN_RNDIS_CONFIG_PARAMETER:
             break;
         case OID_GEN_CURRENT_PACKET_FILTER:
-            if (cmd->InformationBufferLength < sizeof(g_usbd_rndis.net_filter)) {
-                resp->Status = RNDIS_STATUS_INVALID_DATA;
-            } else {
-                uint32_t *filter;
-                /* Parameter starts at offset buf_offset of the req_id field */
-                filter = (uint32_t *)((uint8_t *)&(cmd->RequestId) + cmd->InformationBufferOffset);
-
-                //g_usbd_rndis.net_filter = param->ParameterNameOffset;
-                g_usbd_rndis.net_filter = *(uint32_t *)filter;
-                if (g_usbd_rndis.net_filter) {
-                    g_usbd_rndis.init_state = rndis_data_initialized;
-                } else {
-                    g_usbd_rndis.init_state = rndis_initialized;
-                }
+            if (len < (sizeof(rndis_set_msg_t) + cmd->InformationBufferLength)) {
+                return -1;
             }
+
+            filter = (uint32_t *)(data + sizeof(rndis_set_msg_t));
+            g_usbd_rndis.net_filter = *filter;
             break;
         case OID_GEN_CURRENT_LOOKAHEAD:
             break;
@@ -409,7 +409,10 @@ static int rndis_reset_cmd_handler(uint8_t *data, uint32_t len)
     rndis_reset_cmplt_t *resp;
 
     (void)data;
-    (void)len;
+
+    if (len < sizeof(rndis_reset_cmplt_t)) {
+        return -1;
+    }
 
     resp = ((rndis_reset_cmplt_t *)rndis_encapsulated_resp_buffer);
     resp->MessageType = REMOTE_NDIS_RESET_CMPLT;
@@ -429,7 +432,9 @@ static int rndis_keepalive_cmd_handler(uint8_t *data, uint32_t len)
     rndis_keepalive_msg_t *cmd = (rndis_keepalive_msg_t *)data;
     rndis_keepalive_cmplt_t *resp;
 
-    (void)len;
+    if (len < sizeof(rndis_keepalive_msg_t)) {
+        return -1;
+    }
 
     resp = ((rndis_keepalive_cmplt_t *)rndis_encapsulated_resp_buffer);
     resp->RequestId = cmd->RequestId;
@@ -512,7 +517,7 @@ int usbd_rndis_start_write(uint8_t *buf, uint32_t len)
         return -USB_ERR_BUSY;
     }
 
-    if((len % usbd_get_ep_mps(0, rndis_ep_data[RNDIS_IN_EP_IDX].ep_addr)) == 0) {
+    if ((len % usbd_get_ep_mps(0, rndis_ep_data[RNDIS_IN_EP_IDX].ep_addr)) == 0) {
         /* If the data length is a multiple of the endpoint max packet size, add one byte to indicate the end of the transfer. */
         len += 1;
     }
@@ -628,7 +633,7 @@ int usbd_rndis_set_connect(bool connect)
         return -USB_ERR_NOTCONN;
     }
 
-    if(g_usbd_rndis.set_rsp_get)
+    if (g_usbd_rndis.set_rsp_get)
         return -USB_ERR_BUSY;
 
     rndis_indicate_status_t *resp;
@@ -636,7 +641,7 @@ int usbd_rndis_set_connect(bool connect)
     resp = ((rndis_indicate_status_t *)rndis_encapsulated_resp_buffer);
     resp->MessageType = REMOTE_NDIS_INDICATE_STATUS_MSG;
     resp->MessageLength = sizeof(rndis_indicate_status_t);
-    if(connect) {
+    if (connect) {
         resp->Status = RNDIS_STATUS_MEDIA_CONNECT;
         g_usbd_rndis.link_status = NDIS_MEDIA_STATE_CONNECTED;
     } else {
diff --git a/class/wireless/usbh_rndis.c b/class/wireless/usbh_rndis.c
index 709630cb..2ce075d6 100644
--- a/class/wireless/usbh_rndis.c
+++ b/class/wireless/usbh_rndis.c
@@ -15,9 +15,6 @@
 
 USB_NOCACHE_RAM_SECTION USB_MEM_ALIGNX uint8_t g_rndis_buf[512];
 
-#define CONFIG_USBHOST_RNDIS_ETH_MAX_FRAME_SIZE 1514
-#define CONFIG_USBHOST_RNDIS_ETH_MSG_SIZE       (CONFIG_USBHOST_RNDIS_ETH_MAX_FRAME_SIZE + 44)
-
 static USB_NOCACHE_RAM_SECTION USB_MEM_ALIGNX uint8_t g_rndis_rx_buffer[USB_ALIGN_UP(CONFIG_USBHOST_RNDIS_ETH_MAX_RX_SIZE, CONFIG_USB_ALIGN_SIZE)];
 static USB_NOCACHE_RAM_SECTION USB_MEM_ALIGNX uint8_t g_rndis_tx_buffer[USB_ALIGN_UP(CONFIG_USBHOST_RNDIS_ETH_MAX_TX_SIZE, CONFIG_USB_ALIGN_SIZE)];
 // static USB_NOCACHE_RAM_SECTION USB_MEM_ALIGNX uint8_t g_rndis_inttx_buffer[USB_ALIGN_UP(16, CONFIG_USB_ALIGN_SIZE)];
@@ -83,7 +80,7 @@ static int usbh_rndis_init_msg_transfer(struct usbh_rndis *rndis_class)
     setup->wLength = sizeof(g_rndis_buf);
 
     ret = usbh_control_transfer(rndis_class->hport, setup, (uint8_t *)resp);
-    if (ret < 0) {
+    if (ret < sizeof(rndis_initialize_cmplt_t)) {
         USB_LOG_ERR("init recv error, ret: %d\r\n", ret);
         return ret;
     }
@@ -141,7 +138,7 @@ int usbh_rndis_query_msg_transfer(struct usbh_rndis *rndis_class, uint32_t oid,
     setup->wLength = sizeof(g_rndis_buf);
 
     ret = usbh_control_transfer(rndis_class->hport, setup, (uint8_t *)resp);
-    if (ret < 0) {
+    if (ret < sizeof(rndis_query_cmplt_t)) {
         USB_LOG_ERR("oid:%08x recv error, ret: %d\r\n", (unsigned int)oid, ret);
         return ret;
     }
@@ -198,7 +195,7 @@ static int usbh_rndis_set_msg_transfer(struct usbh_rndis *rndis_class, uint32_t
     setup->wLength = sizeof(g_rndis_buf);
 
     ret = usbh_control_transfer(rndis_class->hport, setup, (uint8_t *)resp);
-    if (ret < 0) {
+    if (ret < sizeof(rndis_set_cmplt_t)) {
         USB_LOG_ERR("oid:%08x recv error, ret: %d\r\n", (unsigned int)oid, ret);
         return ret;
     }
@@ -265,7 +262,7 @@ int usbh_rndis_keepalive(struct usbh_rndis *rndis_class)
     setup->wLength = sizeof(g_rndis_buf);
 
     ret = usbh_control_transfer(rndis_class->hport, setup, (uint8_t *)resp);
-    if (ret < 0) {
+    if (ret < sizeof(rndis_keepalive_cmplt_t)) {
         USB_LOG_ERR("keepalive recv error, ret: %d\r\n", ret);
         return ret;
     }
diff --git a/core/usbh_core.c b/core/usbh_core.c
index 8aa92588..5349feb6 100644
--- a/core/usbh_core.c
+++ b/core/usbh_core.c
@@ -366,7 +366,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
     setup->wLength = 8;
 
     ret = usbh_control_transfer(hport, setup, ep0_request_buffer[hport->bus->busid]);
-    if (ret < 0) {
+    if (ret < 8) {
         USB_LOG_ERR("Failed to get device descriptor,errorcode:%d\r\n", ret);
         goto errout;
     }
@@ -426,7 +426,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
     setup->wLength = USB_SIZEOF_DEVICE_DESC;
 
     ret = usbh_control_transfer(hport, setup, ep0_request_buffer[hport->bus->busid]);
-    if (ret < 0) {
+    if (ret < USB_SIZEOF_DEVICE_DESC) {
         USB_LOG_ERR("Failed to get full device descriptor,errorcode:%d\r\n", ret);
         goto errout;
     }
@@ -450,7 +450,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
     setup->wLength = USB_SIZEOF_CONFIG_DESC;
 
     ret = usbh_control_transfer(hport, setup, ep0_request_buffer[hport->bus->busid]);
-    if (ret < 0) {
+    if (ret < USB_SIZEOF_CONFIG_DESC) {
         USB_LOG_ERR("Failed to get config descriptor,errorcode:%d\r\n", ret);
         goto errout;
     }
@@ -477,7 +477,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
     setup->wLength = wTotalLength;
 
     ret = usbh_control_transfer(hport, setup, ep0_request_buffer[hport->bus->busid]);
-    if (ret < 0) {
+    if (ret < wTotalLength) {
         USB_LOG_ERR("Failed to get full config descriptor,errorcode:%d\r\n", ret);
         goto errout;
     }
@@ -721,7 +721,7 @@ resubmit:
         ret = urb->actual_length;
     }
 
-    if (ret < 0 && (ret != -USB_ERR_TIMEOUT)) {
+    if (ret < sizeof(struct usb_setup_packet) && (ret != -USB_ERR_TIMEOUT)) {
         retry--;
         if (retry > 0) {
             USB_LOG_WRN("Control transfer failed, errorcode %d, retrying...\r\n", ret);
@@ -730,7 +730,7 @@ resubmit:
     }
 
     usb_osal_mutex_give(hport->mutex);
-    return ret;
+    return ret < sizeof(struct usb_setup_packet) ? ret : (ret - sizeof(struct usb_setup_packet));
 }
 
 int usbh_get_string_desc(struct usbh_hubport *hport, uint8_t index, uint8_t *output, uint16_t output_len)
@@ -759,6 +759,13 @@ int usbh_get_string_desc(struct usbh_hubport *hport, uint8_t index, uint8_t *out
     dst = output;
     len = src[0];
 
+    if (ret != len) {
+        USB_LOG_ERR("Get string descriptor failed, expect %d, actual %d\r\n", len, ret);
+        return -USB_ERR_IO;
+    }
+
+    memset(output, 0, output_len);
+
     if (((len - 2) / 2) > output_len) {
         return -USB_ERR_NOMEM;
     }
diff --git a/docs/zh/api/api_host.rst b/docs/zh/api/api_host.rst
index ad93d2fb..393a6ff4 100644
--- a/docs/zh/api/api_host.rst
+++ b/docs/zh/api/api_host.rst
@@ -145,6 +145,20 @@ usbh_find_class_instance
 - **devname**  class 名称
 - **return**  class 结构体句柄
 
+usbh_control_transfer
+""""""""""""""""""""""""""""""""""""
+
+``usbh_control_transfer`` 用来发送控制传输请求。
+
+.. code-block:: C
+
+    int usbh_control_transfer(struct usbh_hubport *hport, struct usb_setup_packet *setup, void *buffer);
+
+- **hport**  hubport 结构体句柄
+- **setup**  控制传输请求结构体指针
+- **buffer**  数据缓冲区指针
+- **return**  实际传输的数据长度或者错误码，不包含 setup 长度
+
 lsusb
 """"""""""""""""""""""""""""""""""""