Skip to content

protect std::io::Take::limit from overflow in read #95040

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
Merged
merged 1 commit into from
Jul 25, 2022
Merged

protect std::io::Take::limit from overflow in read #95040

merged 1 commit into from
Jul 25, 2022

Conversation

@frank-king
Copy link
Contributor

@frank-king frank-king commented Mar 17, 2022 •
edited by Mark-Simulacrum
Loading

Resolves #94981

@rust-highfive
Copy link
Contributor

rust-highfive commented Mar 17, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dtolnay (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2022
@dtolnay dtolnay added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Mar 18, 2022
dtolnay
dtolnay requested changes Mar 18, 2022
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@frank-king frank-king force-pushed the fix/94981 branch 2 times, most recently from dd72dbc to ed528ac Compare March 20, 2022 04:01
@dtolnay
Copy link
Member

dtolnay commented Apr 16, 2022

r? rust-lang/libs

@JohnCSimon
Copy link
Member

JohnCSimon commented May 22, 2022

triage:
@dtolnay - I can't tell if this PR is waiting on review or for the author to do something.

@frank-king
Copy link
Contributor Author

frank-king commented May 23, 2022

@JohnCSimon I think the current state is: first @dtolnay suggested to panic in Take::read instead of silently ignoring the underflow error of subtraction, then I made the change, and now @dtolnay is probably waiting for someone in @rust-lang/libs to confirm this modification.

the8472
the8472 requested changes May 28, 2022
View reviewed changes
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2022
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Jul 24, 2022

I think this is OK and does not need a dedicated T-libs-api FCP, even though it's technically a behavior change. It's hard to imagine code intentionally returning a too large read bytes size from read(), and we've already had two members of T-libs-api sign off on the change, so I think it's probably not necessary.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 24, 2022

📌 Commit 64ac045 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2022
@JohnTitor JohnTitor mentioned this pull request Jul 25, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
@bors
Auto merge of rust-lang#99707 - JohnTitor:rollup-74rb8vq, r=JohnTitor
2fdbf07 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#95040 (protect `std::io::Take::limit` from overflow in `read`)
 - rust-lang#95916 (kmc-solid: Use `libc::abort` to abort a program)
 - rust-lang#99494 (Use non-relocatable code in nofile-limit.rs test)
 - rust-lang#99581 (Improve error messages involving `derive` and `packed`.)
 - rust-lang#99643 (Add `sign-ext` target feature to the WASM target)
 - rust-lang#99659 (Use `VecMap::get` in `ConstraintLocator::check`)
 - rust-lang#99690 (add miri-track-caller to more intrinsic-exposing methods)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0ecbcbb into rust-lang:master Jul 25, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 25, 2022
@dtolnay dtolnay self-assigned this Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaahc yaahc yaahc left review comments

@the8472 the8472 the8472 requested changes

@dtolnay dtolnay dtolnay requested changes

Assignees

@joshtriplett joshtriplett

@dtolnay dtolnay

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.64.0

Development

Successfully merging this pull request may close these issues.

Surprising behavior of Take with misbehaved inner reader

10 participants

@frank-king @rust-highfive @dtolnay @JohnCSimon @Mark-Simulacrum @bors @the8472 @yaahc @joshtriplett @rustbot