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.

Sign up for GitHub

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

Jump to bottom

usbus: check received setup request data amount #17203

Merged
merged 2 commits into from
Nov 17, 2021
Merged

usbus: check received setup request data amount #17203

merged 2 commits into from
Nov 17, 2021

Conversation

bergzand
Copy link
Member

Contribution description

This PR adds two sanity checks to the usbus stack.

  • A check to verify that the amount of data received with a setup request doesn't exceed the amount indicated within the setup request length field
  • A check to ensure that the length of the line coding setup request in the CDC ACM handler matches the expected length.

Testing procedure

Testing that the CDC ACM test still works as expected should be sufficient

Issues/PRs references

None

@bergzand
cdc_acm: Abort line coding request on incorrect size
2ba0222 
This adds a sanity check to the line coding request of the CDC ACM code
to chcek the length parameter in the setup request with the size of the
expected payload struct
@bergzand
usbus_control: check received setup request data amount
4573d5f 
This adds a check to the usbus control stack to ensure that the amount
of data received with a setup request does not exceed the amount
indicated within the setup request
@bergzand bergzand requested a review from dylad November 15, 2021 18:42
@github-actions github-actions bot added Area: sys Area: System Area: USB Area: Universal Serial Bus labels Nov 15, 2021
@dylad dylad added this to the Release 2022.01 milestone Nov 15, 2021
@dylad dylad self-assigned this Nov 15, 2021
@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Nov 15, 2021
dylad
dylad approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.
Changes look good, tested on SAML21-XPRO w/ tests/usbus_cdc_acm_stdio. It still works as expected.

@dylad dylad added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 16, 2021
@dylad dylad mentioned this pull request Nov 16, 2021
Merged
6 tasks
@fjmolinas fjmolinas merged commit 74cdb11 into RIOT-OS:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylad dylad dylad approved these changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

Assignees

@dylad dylad

Labels
Area: sys Area: System Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Milestone
Release 2022.01
Development

Successfully merging this pull request may close these issues.
3 participants
@bergzand @dylad @fjmolinas