Skip to content

#115296 broke include_bytes! on paths with untrustworthy metadata #115458

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

Closed
saethlin opened this issue Sep 1, 2023 · 1 comment · Fixed by #115549
Closed

#115296 broke include_bytes! on paths with untrustworthy metadata #115458

saethlin opened this issue Sep 1, 2023 · 1 comment · Fixed by #115549
Assignees
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@saethlin
Copy link
Member

saethlin commented Sep 1, 2023

In #115296 this diff is subtly broken:

-    fn read_binary_file(&self, path: &Path) -> io::Result<Vec<u8>> {
-        fs::read(path)
+    fn read_binary_file(&self, path: &Path) -> io::Result<Lrc<[u8]>> {
+        let mut file = fs::File::open(path)?;
+        let len = file.metadata()?.len();

+        let mut bytes = Lrc::new_uninit_slice(len as usize);
+        let mut buf = BorrowedBuf::from(Lrc::get_mut(&mut bytes).unwrap());
+        file.read_buf_exact(buf.unfilled())?;
+        // SAFETY: If the read_buf_exact call returns Ok(()), then we have
+        // read len bytes and initialized the buffer.
+        Ok(unsafe { bytes.assume_init() })

because it trusts that the size of the file returned by File::metadata is trustworthy, and it does not need to be. The correct implementation is to call read until EOF is reached.

This can be observed by trying to include_bytes!("/dev/random") (just be sure to use the resultant const).

@saethlin saethlin added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Sep 1, 2023
@saethlin saethlin self-assigned this Sep 1, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 1, 2023
@apiraino
Copy link
Contributor

apiraino commented Sep 4, 2023

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 4, 2023
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 4, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 21, 2023
…jackh726

Fall back to the unoptimized implementation in read_binary_file if File::metadata lies

Fixes rust-lang#115458

r? `@jackh726` because you approved the previous PR
@bors bors closed this as completed in 4fda889 Sep 21, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Sep 21, 2023
Fall back to the unoptimized implementation in read_binary_file if File::metadata lies

Fixes rust-lang/rust#115458

r? `@jackh726` because you approved the previous PR
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Fall back to the unoptimized implementation in read_binary_file if File::metadata lies

Fixes rust-lang/rust#115458

r? `@jackh726` because you approved the previous PR
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Fall back to the unoptimized implementation in read_binary_file if File::metadata lies

Fixes rust-lang/rust#115458

r? `@jackh726` because you approved the previous PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants