Skip to content

Conversation

@joeykraut
Copy link
Member

@joeykraut joeykraut commented Aug 1, 2023

Purpose

This PR adds buffers for partial reads in the network layer of the MpcFabric. This is necessary because tokio::select may cancel a read future, which drops the underlying partially read buffer of the next message without resetting the quic stream's cursor.

This means the next time a read future is created, it will begin from the middle of a message in the quic stream, causing an invalid read or blocking on data that will never be available.

The fix is to buffer partial reads in the QuicTwoPartyNet instance, which is maintained across cancelled futures.

Testing

  • Unit and integration tests pass
  • Replicated the issue with a large circuit in renegade::circuits and verified that this issue was fixed

@joeykraut joeykraut force-pushed the joey/network-cancellation-safety branch from e19282a to 084b165 Compare August 1, 2023 17:55
@joeykraut joeykraut marked this pull request as ready for review August 1, 2023 17:58
@joeykraut joeykraut requested a review from akirillo August 1, 2023 17:58
Copy link

@akirillo akirillo left a comment

Choose a reason for hiding this comment

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

lgtm, left a small q / suggestion

@joeykraut joeykraut force-pushed the joey/network-cancellation-safety branch from 084b165 to a55a636 Compare August 1, 2023 21:05
@joeykraut joeykraut merged commit 7515486 into master Aug 1, 2023
@joeykraut joeykraut deleted the joey/network-cancellation-safety branch August 12, 2023 18:42
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.

3 participants