Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
frank-king:fix/94981
Jul 25, 2022
Merged

protect std::io::Take::limit from overflow in read#95040
bors merged 1 commit into
rust-lang:masterfrom
frank-king:fix/94981

Conversation

@frank-king

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

Copy link
Copy Markdown
Contributor

Resolves#94981

@rust-highfive

rust-highfive commented Mar 17, 2022

Copy link
Copy Markdown
Contributor

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-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2022
@dtolnaydtolnay 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

@dtolnaydtolnay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR!

Comment threadlibrary/std/src/io/mod.rs Outdated
@frank-king
frank-kingforce-pushed the fix/94981 branch 2 times, most recently from dd72dbc to ed528acCompareMarch 20, 2022 04:01
@dtolnay

dtolnay commented Apr 16, 2022

Copy link
Copy Markdown
Member

r? rust-lang/libs

@JohnCSimon

JohnCSimon commented May 22, 2022

Copy link
Copy Markdown

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

@frank-king

frank-king commented May 23, 2022

Copy link
Copy Markdown
ContributorAuthor

@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
Comment threadlibrary/std/src/io/tests.rs Outdated
Comment threadlibrary/std/src/io/mod.rs Outdated
Comment threadlibrary/std/src/io/tests.rs Outdated
@JohnCSimonJohnCSimon 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

Mark-Simulacrum commented Jul 24, 2022

Copy link
Copy Markdown
Member

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

bors commented Jul 24, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 64ac045 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@borsbors 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
@JohnTitorJohnTitor 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 ofrust-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:masterJul 25, 2022
@rustbotrustbot added this to the 1.64.0 milestone Jul 25, 2022
@dtolnaydtolnay self-assigned this Mar 24, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yaahcyaahcyaahc left review comments
@the8472the8472the8472 requested changes
@dtolnaydtolnaydtolnay requested changes

Assignees

@joshtriplettjoshtriplett

@dtolnaydtolnay

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-libsRelevant 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