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

fix(epoll): buffer overflow when GSO off #4717

Open
wants to merge 1 commit into
base: release/2.3
Choose a base branch
from

Conversation

qzhuyan
Copy link
Contributor

@qzhuyan qzhuyan commented Dec 19, 2024

Description

Fix buffer overflow that SendData buffer size could not be checked correctly in CxPlatSendDataFinalizeSendBuffer when SendData->SegmentSize is 0.

This issue was found in a env where GSO is off, mtu size 1400 and sending large payload (size: 780038) + Linux epoll.
It is hard to reproduce in other env.

this is a quick fix and hurt performance, I think for long term the CxPlatSendDataFinalizeSendBuffer should be aware of the allocating buffer size.

Testing

_

Documentation

_

@qzhuyan qzhuyan requested a review from a team as a code owner December 19, 2024 12:46
@nibanks nibanks added OS: Ubuntu external Proposed by non-MSFT Bug: Platform A code bug in platform/TLS specific code. labels Dec 19, 2024
qzhuyan added a commit to qzhuyan/quic that referenced this pull request Dec 19, 2024
- buffer overflow when tracing
- buffer overflow when GSO off, Linux epoll, large payload
  microsoft/msquic#4717
qzhuyan added a commit to qzhuyan/quic that referenced this pull request Dec 19, 2024
- buffer overflow when tracing
- buffer overflow when GSO off, Linux epoll, large payload
  microsoft/msquic#4717
@qzhuyan
Copy link
Contributor Author

qzhuyan commented Dec 19, 2024

More info of ASAN check (without the fix) here:

emqx/quic#334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Platform A code bug in platform/TLS specific code. external Proposed by non-MSFT OS: Ubuntu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants