Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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