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

pic32 sanity fixes #1451

Merged
merged 3 commits into from May 24, 2022
Merged

pic32 sanity fixes #1451

merged 3 commits into from May 24, 2022

Conversation

kasjer
Copy link
Collaborator

@kasjer kasjer commented May 1, 2022

Describe the PR
For PIC32 OUT packets that were sent from host and not handled
fast enough could lead to memory corruption.
When transfer was complete OUT endpoint was left open for
host to send more data (which is fine and allow for faster transfer).
however if data packet arrived before new transfer was scheduled
code was reading endpoint and writing past buffer that was filled
with previous transfer data.
To prevent such scenario RX interrupt is disabled when transfer is
finished (data can still arrived at endpoint).
Interrupt handler now handles only enabled RX transfers.
When new transfer starts it will enable interrupt (as it was doing
so far).

Two first commits are just to improve error detection.
Third fixes overwrite problem.
Additional context
This was detected when MSC device was used.

kasjer added 3 commits May 1, 2022 14:05
_mips is provided by xc32-gcc
TU_ASSERTS added to detect transfer inconsistency.
When transfer was finished rx_fifo_read() read all that
was to read RXPKTRDY was cleared allowing next packet to
be received.
Then xfer_complete was called.
Interrupt for OUT endpoint was left enable, that would not
be a problem if data was handled fast and new transfer was
scheduled.
For MSC when host sends a lot of data this interrupt that was
enabled could cause epn_handle_rx_int() to be called after
transfer was completed and next was not scheduled yet.
Without TU_ASSERT that was added to detect this, incoming
data was written past buffer provided by user code resulting
in random memory corruption.

This just blocks RX interrupt when transfer is finished,
and also only unmasked rx interrupts are handled.
@kasjer kasjer mentioned this pull request May 23, 2022
Copy link
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

sorry, this is kind of falling off my radar after heavily concentrated on other works. Please remind me after a week or without response, since I can get lost with other works.

@hathach hathach merged commit 0a4a28a into hathach:master May 24, 2022
@kasjer kasjer deleted the kasjer/pic32-sanity-fixes branch May 24, 2022 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants