STMicroelectronics / STM32CubeWB

Full Firmware Package for the STM32WB series: HAL+LL drivers, CMSIS, BSP, MW, plus a set of Projects (examples and demos) running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits).
https://www.st.com/en/embedded-software/stm32cubewb.html
Other
229 stars 139 forks source link

Race condition between (s)hci_cmd_resp_wait and (s)hci_cmd_resp_release #79

Closed ThiemoVanEngelen closed 1 year ago

ThiemoVanEngelen commented 1 year ago

Describe the set-up

Describe the bug The current code in shci_tl.c:shci_send() is roughly the following:

...
shciContext.io.Send(0,0);
shci_cmd_resp_wait(SHCI_TL_DEFAULT_TIMEOUT);

shci_cmd_resp_wait contains the following code:

  CmdRspStatusFlag = SHCI_TL_CMD_RESP_WAIT;
  while(CmdRspStatusFlag != SHCI_TL_CMD_RESP_RELEASE);

so it waits until CmdRspStatusFlag == SHCI_TL_CMD_RESP_RELEASE. This should happen in shci_cmd_resp_release: CmdRspStatusFlag = SHCI_TL_CMD_RESP_RELEASE;

Now when an interrupt occurs between the shciContext.io.Send(0,0); and the CmdRspStatusFlag = SHCI_TL_CMD_RESP_WAIT;, it can happen that CPU2 is already done with the command, causing CPU1 to execute shci_cmd_resp_release (thus CmdRspStatusFlag = SHCI_TL_CMD_RESP_RELEASE;) before executing CmdRspStatusFlag = SHCI_TL_CMD_RESP_WAIT in shci_cmd_resp_wait, which then forever waits until CmdRspStatusFlag == SHCI_TL_CMD_RESP_RELEASE, because it overwrites the SHCI_TL_CMD_RESP_RELEASE.

This is more or less the same in hci_tl.c for the hci equivalents.

How To Reproduce

  1. Indicate the global behavior of your application project. No application written that shows this behavior. This is found by code inspection.

  2. The modules that you suspect to be the cause of the problem (Driver, BSP, MW ...). STM32_WPAN

  3. The use case that generates the problem.

  4. How we can reproduce the problem. Generate an interrupt right after the Send that takes enough time for CPU2 to finish handling the command.

Additional context One solution, that might also help in other situation where the default/__WEAK implementation is not used, is to add a (s)hci_cmd_resp_prepare function that is called before shciContext.io.Send(0,0); is executed. For the default/__WEAK implementation, this could then execute CmdRspStatusFlag = HCI_TL_CMD_RESP_WAIT;, making sure that CmdRspStatusFlag already has the correct value before shciContext.io.Send(0,0); is actually executed. Other implementations could also use this function to make sure that they are correctly setup to wait for the response.

Screenshots

TOUNSTM commented 1 year ago

Hello @ThiemoVanEngelen,

Thank you for this report. We will get back to you as soon as we analyze it further. This may take some time. Thank you for your comprehension.

With regards,

RJMSTM commented 1 year ago

ST Internal Reference: 154227

RJMSTM commented 1 year ago

Hello,

This issue has been fixed in the frame of version v1.17.0 of the STM32CubeWB. Please allow me then to close this thread.

With regards, Rania