espressif/esp-idf
1
0
Fork 1
mirror of https://github.com/espressif/esp-idf.git synced 2025-07-30 18:57:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'feature/usb_host_inflight_transfer_check_v5.0' into 'release/v5.0'

Browse Source 
USB Host: Fix usbh_dev_open(), Fix usbh_ep_alloc(), Add USB Host Library check to prevent resubmitting inflight transfers, Don't access uninit pointer if usbh_install fails (backport v5.0)

See merge request espressif/esp-idf!20953
This commit is contained in:
morris
2022-12-05 10:08:32 +08:00
parent 75c3a69feb 823c8d4e77
commit a3275f2b9e
4 changed files with 36 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
components/usb/private_include/usb_private.h
View File

@ -38,12 +38,13 @@ _Static_assert(sizeof(usb_transfer_dummy_t) == sizeof(usb_transfer_t), "usb_tran
struct urb_s{ struct urb_s{
TAILQ_ENTRY(urb_s) tailq_entry; TAILQ_ENTRY(urb_s) tailq_entry;
//HCD handler pointer and variables. Must be initialized to NULL and 0 respectively //HCD Layer: Handler pointer and variables. Must be initialized to NULL and 0 respectively
void *hcd_ptr; void *hcd_ptr;
uint32_t hcd_var; uint32_t hcd_var;
//Host Driver layer handler //Host Lib Layer:
void *usb_host_client; //Currently only used when submitted to shared pipes (i.e., Device default pipes) void *usb_host_client; //Currently only used when submitted to shared pipes (i.e., Device default pipes)
size_t usb_host_header_size; //USB Host may need the data buffer to have a transparent header size_t usb_host_header_size; //USB Host may need the data buffer to have a transparent header
bool usb_host_inflight; //Debugging variable, used to prevent re-submitting URBs already inflight
//Public transfer structure. Must be last due to variable length array //Public transfer structure. Must be last due to variable length array
usb_transfer_t transfer; usb_transfer_t transfer;
}; };

8
components/usb/test/usb_host/msc_client_async_seq.c
View File

@ -192,6 +192,8 @@ void msc_client_async_seq_task(void *arg)
xfer_out->num_bytes = sizeof(usb_setup_packet_t); xfer_out->num_bytes = sizeof(usb_setup_packet_t);
xfer_out->bEndpointAddress = 0; xfer_out->bEndpointAddress = 0;
TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out)); TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out));
//Test that an inflight control transfer cannot be resubmitted
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit_control(msc_obj.client_hdl, xfer_out));
//Next stage set from transfer callback //Next stage set from transfer callback
break; break;
} }
@ -201,6 +203,8 @@ void msc_client_async_seq_task(void *arg)
xfer_out->num_bytes = sizeof(mock_msc_bulk_cbw_t); xfer_out->num_bytes = sizeof(mock_msc_bulk_cbw_t);
xfer_out->bEndpointAddress = MOCK_MSC_SCSI_BULK_OUT_EP_ADDR; xfer_out->bEndpointAddress = MOCK_MSC_SCSI_BULK_OUT_EP_ADDR;
TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_out)); TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_out));
//Test that an inflight transfer cannot be resubmitted
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_out));
//Next stage set from transfer callback //Next stage set from transfer callback
break; break;
} }
@ -209,6 +213,8 @@ void msc_client_async_seq_task(void *arg)
xfer_in->num_bytes = usb_round_up_to_mps(MOCK_MSC_SCSI_SECTOR_SIZE * msc_obj.test_param.num_sectors_per_xfer, MOCK_MSC_SCSI_BULK_EP_MPS); xfer_in->num_bytes = usb_round_up_to_mps(MOCK_MSC_SCSI_SECTOR_SIZE * msc_obj.test_param.num_sectors_per_xfer, MOCK_MSC_SCSI_BULK_EP_MPS);
xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR; xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR;
TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in)); TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in));
//Test that an inflight transfer cannot be resubmitted
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_in));
//Next stage set from transfer callback //Next stage set from transfer callback
break; break;
} }
@ -217,6 +223,8 @@ void msc_client_async_seq_task(void *arg)
xfer_in->num_bytes = usb_round_up_to_mps(sizeof(mock_msc_bulk_csw_t), MOCK_MSC_SCSI_BULK_EP_MPS); xfer_in->num_bytes = usb_round_up_to_mps(sizeof(mock_msc_bulk_csw_t), MOCK_MSC_SCSI_BULK_EP_MPS);
xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR; xfer_in->bEndpointAddress = MOCK_MSC_SCSI_BULK_IN_EP_ADDR;
TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in)); TEST_ASSERT_EQUAL(ESP_OK, usb_host_transfer_submit(xfer_in));
//Test that an inflight transfer cannot be resubmitted
TEST_ASSERT_EQUAL(ESP_ERR_NOT_FINISHED, usb_host_transfer_submit(xfer_in));
//Next stage set from transfer callback //Next stage set from transfer callback
break; break;
} }

27
components/usb/usb_host.c
View File

@ -438,8 +438,8 @@ assign_err:
hub_err: hub_err:
ESP_ERROR_CHECK(usbh_uninstall()); ESP_ERROR_CHECK(usbh_uninstall());
usbh_err: usbh_err:
if (p_host_lib_obj->constant.phy_handle) { if (host_lib_obj->constant.phy_handle) {
ESP_ERROR_CHECK(usb_del_phy(p_host_lib_obj->constant.phy_handle)); ESP_ERROR_CHECK(usb_del_phy(host_lib_obj->constant.phy_handle));
} }
phy_err: phy_err:
alloc_err: alloc_err:
@ -591,6 +591,8 @@ static void _handle_pending_ep(client_t *client_obj)
//Dequeue all URBs and run their transfer callback //Dequeue all URBs and run their transfer callback
urb_t *urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl); urb_t *urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl);
while (urb != NULL) { while (urb != NULL) {
//Clear the transfer's inflight flag to indicate the transfer is no longer inflight
urb->usb_host_inflight = false;
urb->transfer.callback(&urb->transfer); urb->transfer.callback(&urb->transfer);
num_urb_dequeued++; num_urb_dequeued++;
urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl); urb = hcd_urb_dequeue(ep_obj->constant.pipe_hdl);
@ -748,6 +750,8 @@ esp_err_t usb_host_client_handle_events(usb_host_client_handle_t client_hdl, Tic
TAILQ_REMOVE(&client_obj->dynamic.done_ctrl_xfer_tailq, urb, tailq_entry); TAILQ_REMOVE(&client_obj->dynamic.done_ctrl_xfer_tailq, urb, tailq_entry);
client_obj->dynamic.num_done_ctrl_xfer--; client_obj->dynamic.num_done_ctrl_xfer--;
HOST_EXIT_CRITICAL(); HOST_EXIT_CRITICAL();
//Clear the transfer's inflight flag to indicate the transfer is no longer inflight
urb->usb_host_inflight = false;
//Call the transfer's callback //Call the transfer's callback
urb->transfer.callback(&urb->transfer); urb->transfer.callback(&urb->transfer);
HOST_ENTER_CRITICAL(); HOST_ENTER_CRITICAL();
@ -1298,6 +1302,9 @@ esp_err_t usb_host_transfer_submit(usb_transfer_t *transfer)
USB_EP_DESC_GET_XFERTYPE(ep_obj->constant.ep_desc), USB_EP_DESC_GET_XFERTYPE(ep_obj->constant.ep_desc),
USB_EP_DESC_GET_MPS(ep_obj->constant.ep_desc), USB_EP_DESC_GET_MPS(ep_obj->constant.ep_desc),
transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK), ESP_ERR_INVALID_ARG); transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_DIR_MASK), ESP_ERR_INVALID_ARG);
//Check that we are not submitting a transfer already inflight
HOST_CHECK(!urb_obj->usb_host_inflight, ESP_ERR_NOT_FINISHED);
urb_obj->usb_host_inflight = true;
HOST_ENTER_CRITICAL(); HOST_ENTER_CRITICAL();
ep_obj->dynamic.num_urb_inflight++; ep_obj->dynamic.num_urb_inflight++;
HOST_EXIT_CRITICAL(); HOST_EXIT_CRITICAL();
@ -1317,6 +1324,7 @@ hcd_err:
HOST_ENTER_CRITICAL(); HOST_ENTER_CRITICAL();
ep_obj->dynamic.num_urb_inflight--; ep_obj->dynamic.num_urb_inflight--;
HOST_EXIT_CRITICAL(); HOST_EXIT_CRITICAL();
urb_obj->usb_host_inflight = false;
err: err:
return ret; return ret;
} }
@ -1324,6 +1332,7 @@ err:
esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl, usb_transfer_t *transfer) esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl, usb_transfer_t *transfer)
{ {
HOST_CHECK(client_hdl != NULL && transfer != NULL, ESP_ERR_INVALID_ARG); HOST_CHECK(client_hdl != NULL && transfer != NULL, ESP_ERR_INVALID_ARG);
//Check that control transfer is valid //Check that control transfer is valid
HOST_CHECK(transfer->device_handle != NULL, ESP_ERR_INVALID_ARG); //Target device must be set HOST_CHECK(transfer->device_handle != NULL, ESP_ERR_INVALID_ARG); //Target device must be set
usb_device_handle_t dev_hdl = transfer->device_handle; usb_device_handle_t dev_hdl = transfer->device_handle;
@ -1333,8 +1342,18 @@ esp_err_t usb_host_transfer_submit_control(usb_host_client_handle_t client_hdl,
HOST_CHECK(transfer_check(transfer, USB_TRANSFER_TYPE_CTRL, dev_info.bMaxPacketSize0, xfer_is_in), ESP_ERR_INVALID_ARG); HOST_CHECK(transfer_check(transfer, USB_TRANSFER_TYPE_CTRL, dev_info.bMaxPacketSize0, xfer_is_in), ESP_ERR_INVALID_ARG);
//Control transfers must be targeted at EP 0 //Control transfers must be targeted at EP 0
HOST_CHECK((transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK) == 0, ESP_ERR_INVALID_ARG); HOST_CHECK((transfer->bEndpointAddress & USB_B_ENDPOINT_ADDRESS_EP_NUM_MASK) == 0, ESP_ERR_INVALID_ARG);
//Save client handle into URB
urb_t *urb_obj = __containerof(transfer, urb_t, transfer); urb_t *urb_obj = __containerof(transfer, urb_t, transfer);
//Check that we are not submitting a transfer already inflight
HOST_CHECK(!urb_obj->usb_host_inflight, ESP_ERR_NOT_FINISHED);
urb_obj->usb_host_inflight = true;
//Save client handle into URB
urb_obj->usb_host_client = (void *)client_hdl; urb_obj->usb_host_client = (void *)client_hdl;
return usbh_dev_submit_ctrl_urb(dev_hdl, urb_obj);
esp_err_t ret;
ret = usbh_dev_submit_ctrl_urb(dev_hdl, urb_obj);
if (ret != ESP_OK) {
urb_obj->usb_host_inflight = false;
}
return ret;
} }

4
components/usb/usbh.c
View File

@ -536,7 +536,7 @@ esp_err_t usbh_dev_open(uint8_t dev_addr, usb_device_handle_t *dev_hdl)
goto exit; goto exit;
} }
} }
TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_idle_tailq, dynamic.tailq_entry) { TAILQ_FOREACH(dev_obj, &p_usbh_obj->dynamic.devs_pending_tailq, dynamic.tailq_entry) {
if (dev_obj->constant.address == dev_addr) { if (dev_obj->constant.address == dev_addr) {
found_dev_obj = dev_obj; found_dev_obj = dev_obj;
goto exit; goto exit;
@ -783,7 +783,7 @@ esp_err_t usbh_ep_alloc(usb_device_handle_t dev_hdl, usbh_ep_config_t *ep_config
if (is_in && dev_obj->mux_protected.ep_in[addr - 1] == NULL) { //Is an IN EP if (is_in && dev_obj->mux_protected.ep_in[addr - 1] == NULL) { //Is an IN EP
dev_obj->mux_protected.ep_in[addr - 1] = pipe_hdl; dev_obj->mux_protected.ep_in[addr - 1] = pipe_hdl;
assigned = true; assigned = true;
} else if (dev_obj->mux_protected.ep_out[addr - 1] == NULL) { //Is an OUT EP } else if (!is_in && dev_obj->mux_protected.ep_out[addr - 1] == NULL) { //Is an OUT EP
dev_obj->mux_protected.ep_out[addr - 1] = pipe_hdl; dev_obj->mux_protected.ep_out[addr - 1] = pipe_hdl;
assigned = true; assigned = true;
} }