Buffer overrun with USBH_CDC_Receive

[Rimpampa]

Introduction

I'm working with a F4 family ST MCU and I'm using this middleware to communicate with a CDC USB device.

The code involved is somewhat like this:

extern USBH_HandleTypeDef hUsbHostFS;

bool rx_end = true;

void recv(uint8_t data[], uint32_t size) {
  USBH_CDC_Receive(&hUsbHostFS, data, size);
  rx_end = false;
}

void USBH_CDC_ReceiveCallback(USBH_HandleTypeDef *phost) {
  rx_end = true;
}

void recv_all(uint8_t data[], uint32_t size) {
    uint32_t at = 0;
    while(at < size) {
        recv(data + at, size - at);
        while(!rx_end);
        at += USBH_CDC_GetLastReceivedDataSize(&hUsbHostFS);
    }
}

(for the scope of this issue I've excluded many important parts like synchronization of the global state access and error handling)

The problem

That code works fine most of the times but sometimes it causes a buffer overrun when the data buffer is too small (or the at index is too close to the end of data).

This becomes obvious when I call recv_all(buffer, 0) and see that data is still being received.

Analysis

I want to state right away that I'm beginner and this could be all wrong as I haven't had the time to dig deeper into this problem.

The main cause of this problem I think resides in the implementation of CDC_ProcessReception file.

We can see that USBH_CDC_Receive uses the parameter its given to set the pRxData and RxDataLength fields:

CDC_Handle->pRxData = pbuff;
CDC_Handle->RxDataLength = length;

In the CDC_ProcessReception, though, we can see this:

USBH_BulkReceiveData(
    phost,
    CDC_Handle->pRxData,
    CDC_Handle->DataItf.InEpSize,
    CDC_Handle->DataItf.InPipe
);

The problem with that is that it's not even considering the RxDataLength field, which in turn means that if you call USBH_CDC_Receive with a buffer that is smaller than CDC_Handle->DataItf.InEpSize undefined behaviour will be generated.

One important note is that CDC_ProcessTransmission handles this problem:

if(CDC_Handle->TxDataLength > CDC_Handle->DataItf.OutEpSize) {
    USBH_BulkSendData(
        phost,
        CDC_Handle->pTxData,
        CDC_Handle->DataItf.OutEpSize,
        CDC_Handle->DataItf.OutPipe,
        1U
    );
} else {
    USBH_BulkSendData(
        phost,
        CDC_Handle->pTxData,
        (uint16_t)CDC_Handle->TxDataLength,
        CDC_Handle->DataItf.OutPipe,
        1U
    );
}

Suggestion

If this is expected I feel like it should be explained better in the documentation as I couldn't find any warning about this, but if, instead, this is a problem then I would like to know if a patch could be available in the near future.

[ALABSTM]

Hi @Rimpampa,

Please excuse this delayed reply. Your point seems very relevant. It has been forwarded to our development teams. I will keep you informed.

With regards,

[ALABSTM]

ST Internal Reference: 173367

[ALABSTM]

Hi @Rimpampa,

I eventually got the answer to this issue you raised. According to our development teams, the case is already handled and there is no need to any further update of the CDC_ProcessReception() function.

Below the details:

• This maximal packet size to be received shall never exceed the endpoint's supported maximal packet size.
• This is ensured during the endpoint's descriptor parsing step by the function USBH_ParseEPDesc().
• Should this condition not be satisfied, an error code is returned and no data reception should occur, as shown be the code snippets below.

https://github.com/STMicroelectronics/stm32-mw-usb-host/blob/9d46e44c858a77bf126920b7bcc258d59afa89b1/Core/Src/usbh_ctlreq.c#L578-L583

https://github.com/STMicroelectronics/stm32-mw-usb-host/blob/9d46e44c858a77bf126920b7bcc258d59afa89b1/Core/Src/usbh_ctlreq.c#L617-L624

• Otherwise, the data should be received in chunks, the size of each should not exceed the endpoint's supported maximal packet size.

---

• Now, it is up to the application designer to ensure that the endpoint's reception buffer is large enough to avoid an overflow at packet reception. Typically, the size of this buffer should be a multiple of the endpoint's supported maximal packet size.
• This is why the buffer size of the IN endpoint is passed as an argument to the CDC_ProcessReception() function, as you pointed out and as shown by the line of code below.

https://github.com/STMicroelectronics/stm32-mw-usb-host/blob/3a524c2f185ac2f193cf3edd0b3f83ed064f3a5c/Class/CDC/Src/usbh_cdc.c#L738

I hope this clarifies the point. Please allow me to close this issue. Thank you for your comprehension and thank you again for having reported.

With regards,