Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack hardware wake-up support checking #78

Closed
TheSilentDawn opened this issue Oct 14, 2020 · 5 comments
Closed

Lack hardware wake-up support checking #78

TheSilentDawn opened this issue Oct 14, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system mw Middleware-related issue or pull-request. usb USB-related (host or device) issue or pull-request
Milestone

Comments

@TheSilentDawn
Copy link

TheSilentDawn commented Oct 14, 2020

Describe the set-up

  • Software:
    • STM32Cube MCU & MPU Packages
  • Version:
    • STM32Cube_FW_H7_V1.8.0
  • Verification Hardware Platform:
    • STM32H7B3

Describe the bug

  • Function:

    • static void USBH_ParseCfgDesc(USBH_CfgDescTypeDef *cfg_desc, uint8_t *buf, uint16_t length)
  • Location:

  • Type:

    • Denial-of-Service.
  • Result:

    • The system will hang when trying to set a remote wake-up feature.
  • Description:

    • The function USBH_ParseCfgDesc() parses the configuration descriptor, interface descriptor, and endpoint descriptor by input data from a USB device.
    • And it set the variable cfg_desc->bmAttributes by the input data from the USB device. This variable will be used as part of a judgment in the function USBH_Process() as shown in
      if ((phost->device.CfgDesc.bmAttributes) & (1U << 5))
      . With a malformed value, the remote wakeup may be enabled as shown in
      if ((phost->device.CfgDesc.bmAttributes) & (1U << 5))
      . If the hardware doesn’t support this feature, the system will hang due to a FAIL return value by the function USBH_HandleControl().

How To Reproduce

  1. Running MSC_Standalone application on the STM32H7B3I platform

  2. Plug a USB disk

  3. Use the attached Bug4.txt to replace the USB device packet. Bug4.txt

Additional context

  • To patch it, the program should check if the hardware supports a remote wake-up feature.
@ALABSTM
Copy link
Contributor

ALABSTM commented Nov 24, 2020

Hi @TheSilentDawn,

Thank you for this other report. If I understood the idea we should ensure that the software embedded within the host checks whether the value bmAttributes provided by the device is correct? Which reference should the host base itself on to perform such a check? Should we not assume the data provided by the device is correct?

Otherwise, when to stop checking? In case the host has a reference to base itself on, then would it not also make sense to check the validity of this reference too?

I hope my questions make sense. My goal is to be sure I understood correctly your point before reporting it to our development teams. I am looking forward to reading your reply. Thank you again.

With regards,

@TheSilentDawn
Copy link
Author

TheSilentDawn commented Nov 25, 2020

Hi @ALABSTM,
No, I think the software embedded within the host should check if the hardware or the board supports or enables the wake-up feature. If not, the firmware shouldn't check bmAttributes as the current logic which will lead the whole system to hang. And to patch it, the wake-up feature checking codes should be added before the bmAttributes is accessed in the function and position reported upper. If anything not clear, please let me know. Thanks for your help.^_^

@ALABSTM
Copy link
Contributor

ALABSTM commented Dec 2, 2020

Hi @TheSilentDawn,

Please allow me this couple of questions:

  • Is there any way for the host to check whether the device supports the wake-up feature other than the bmAttributes read from the device descriptor?
  • You mentioned "the wake-up feature checking codes". Are you referring to some piece of code already present in our source files that should be moved before the reading of the bmAttributes?

Thank you,

@ALABSTM ALABSTM moved this from To do to Assigned in stm32cube-mcu-fw-dashboard Dec 2, 2020
@ALABSTM ALABSTM added the mw Middleware-related issue or pull-request. label Dec 2, 2020
@ALABSTM ALABSTM added the enhancement New feature or request label Dec 15, 2020
@ALABSTM ALABSTM added the usb USB-related (host or device) issue or pull-request label Jan 18, 2021
@ALABSTM ALABSTM moved this from Assigned to In progress in stm32cube-mcu-fw-dashboard Jan 18, 2021
@ALABSTM ALABSTM added the internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system label Jan 18, 2021
@ALABSTM
Copy link
Contributor

ALABSTM commented Jan 18, 2021

ST Internal Reference: 99173

@ALABSTM
Copy link
Contributor

ALABSTM commented Mar 14, 2022

Hi @TheSilentDawn,

Hope you're fine. Just to inform you the fix has been published in the frame of v1.10.0 release.

With regards,

@ALABSTM ALABSTM closed this as completed Mar 14, 2022
stm32cube-mcu-fw-dashboard automation moved this from To release to Done Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system mw Middleware-related issue or pull-request. usb USB-related (host or device) issue or pull-request
Projects
Development

No branches or pull requests

2 participants