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 buffer overrun in bootstrap and (test-only) symlink_junction #109960

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

thomcc
Copy link
Member

@thomcc thomcc commented Apr 5, 2023

I don't think these can be hit in practice, due to their inputs being valid paths. It's also not security-sensitive code, but just... bad vibes.

I think this is still not really the right way to do this (in terms of path correctness), but is no worse than it was.

r? @ChrisDenton

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 5, 2023
@rustbot
Copy link
Collaborator

rustbot commented Apr 5, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@jyn514
Copy link
Member

jyn514 commented Apr 5, 2023

Can we switch to using docs.rs/junction instead? it makes me nervous to have all this unsafe code without tests ...

@thomcc
Copy link
Member Author

thomcc commented Apr 5, 2023

Can we switch to using docs.rs/junction instead

For bootstrap that seems reasonable, done.

For std I'm less sure. Even if we can (not really sure -- for a dev-dep I think we don't need it to use special rustc-dep-of-std stuff? Although maybe it would need to be in integration tests to avoid that...), deps in std (even dev-deps) have a way of causing headaches -- I'll leave it up to the reviewer.

@albertlarsan68
Copy link
Member

r=me on the bootstrap changes

@ChrisDenton
Copy link
Member

I'll look at these changes in a bit but I'm definitely in favour of avoiding headaches 🙂. I think the ultimate solution is for me to finally write an ACP (which I'll do now before I forget), fix up the std function with tests and finally get it stabilized.

@ChrisDenton
Copy link
Member

Ok this looks like an improvement, even if it's only avoiding an unlikely edge case. The new dependency looks fine for bootstrap though hopefully we can unify this code in a future libstd.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 5, 2023

📌 Commit 42e38e8 has been approved by ChrisDenton

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 Apr 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 7, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#109806 (Workaround rust-lang#109797 on windows-gnu)
 - rust-lang#109957 (diagnostics: account for self type when looking for source of unsolved type variable)
 - rust-lang#109960 (Fix buffer overrun in bootstrap and (test-only) symlink_junction)
 - rust-lang#110013 (Label `non_exhaustive` attribute on privacy errors from non-local items)
 - rust-lang#110016 (Run collapsed GUI test in mobile mode as well)
 - rust-lang#110022 (fix: fix regression in rust-lang#109203)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 17ed06a into rust-lang:master Apr 7, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 7, 2023
aliemjay added a commit to aliemjay/rust that referenced this pull request Apr 15, 2023
bootstrap: drop some windows features

They became unused after rust-lang#109960
aliemjay added a commit to aliemjay/rust that referenced this pull request Apr 15, 2023
bootstrap: drop some windows features

They became unused after rust-lang#109960
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2023
bootstrap: drop some windows features

They became unused after rust-lang#109960
oli-obk pushed a commit to oli-obk/miri that referenced this pull request Apr 17, 2023
bootstrap: drop some windows features

They became unused after rust-lang/rust#109960
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants