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

Avoid uint32 overflow when applying initial window size setting #1268

Merged

Conversation

ahmadsherif
Copy link
Contributor

If s.Val is less than t.streamSendQuota, subtraction can result in a very big number due to overflow (both fields are uint32), effectively giving the stream a lot of quota to send data while its peer is expecting otherwise.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@ahmadsherif
Copy link
Contributor Author

My employer (GitLab) signed a CLA.

@mehrdada
Copy link
Member

mehrdada commented May 31, 2017

Thanks a lot for your PR. Last night, while debugging #1192, I concluded that we have some overflows that are causing the issue, so I'm glad that it's fixed automatically by someone else 👍 . I think we need to look more closely at all the flow-control arithmetics for potential overflows :)

@mehrdada
Copy link
Member

@ahmadsherif Do you happen to know when GitLab signed the CLA? I am unable to find "GitLab Inc." as a signer of Google CLA.

@ahmadsherif
Copy link
Contributor Author

@mehrdada It was yesterday around 16:30 UTC.

cc/ @jacobvosmaer

@ahmadsherif ahmadsherif changed the title Avoid int32 overflow when applying initial window size setting Avoid uint32 overflow when applying initial window size setting May 31, 2017
@mehrdada
Copy link
Member

@ahmadsherif There seems to be no record of GitLab Inc. ever signing Google's CLA. Can you follow up with folks on your side? I am interested in merging this as soon as possible if we can figure out the CLA issues. Thanks!

@jacobvosmaer
Copy link

I submitted the CLA on behalf of GitLab Inc. earlier this week but I got rejected because I'm not an executive. We will have someone else in our org resubmit the CLA.

@mehrdada mehrdada merged commit a8cd0c1 into grpc:master Jun 1, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants