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

IsZero for raw pointers is unsound #135338

Closed
joboet opened this issue Jan 10, 2025 · 4 comments · Fixed by #135339
Closed

IsZero for raw pointers is unsound #135338

joboet opened this issue Jan 10, 2025 · 4 comments · Fixed by #135339
Assignees
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@joboet
Copy link
Member

joboet commented Jan 10, 2025

The IsZero trait is used to specialize vec![val; n] to use allocate_zeroed when the val being duplicated is zero. But in the case of raw pointers, this is not correct as the bytes returned by allocate_zeroed do not have the same provenance as val. Thus, the following code triggers undefined behaviour (playground) when it shouldn't

let ptr = std::ptr::from_ref(&42);
let zero = ptr.with_addr(0);
let roundtripped = vec![zero; 1].pop().unwrap();
let new = roundtripped.with_addr(ptr.addr());
unsafe { new.read() };
@joboet joboet added A-strict-provenance Area: Strict provenance for raw pointers C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jan 10, 2025
@joboet joboet self-assigned this Jan 10, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 10, 2025
@saethlin saethlin removed A-strict-provenance Area: Strict provenance for raw pointers needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 10, 2025
@saethlin
Copy link
Member

I removed the strict-provenance label because this can be reproduced without using the strict provenance APIs:

fn main() {
    let ptr = std::ptr::from_ref(&42u8);
    let zero = ptr.wrapping_sub(ptr as usize);
    let roundtripped = vec![zero; 1].pop().unwrap();
    let new = roundtripped.wrapping_add(ptr as usize);
    unsafe { new.read() };
}

@scottmcm
Copy link
Member

Pondering: We have IsZero for Option<&T> and Option<Box<T>> as well.

I guess those are fine because the None for those isn't allowed to carry provenance, even though it's guaranteed to "be" a null pointer?

@lolbinarycat
Copy link
Contributor

One difference is that 0 may actually be a meaningful address for a raw pointer, as is the case with interrupt vectors on some platforms.

@hanna-kruppe
Copy link
Contributor

Rust does not support that: a null pointer is never valid and the null pointer has address zero. If there’s something at address zero on the platform you target, you have to access it with inline assembly. See previous discussion at rust-lang/unsafe-code-guidelines#29

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 10, 2025
alloc: remove unsound `IsZero` for raw pointers

Fixes rust-lang#135338
@bors bors closed this as completed in 4426e9a Jan 11, 2025
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 13, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 20, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 20, 2025
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Feb 20, 2025
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Feb 20, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 21, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 21, 2025
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Feb 22, 2025
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Feb 22, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Feb 22, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Feb 22, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 3, 2025
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Mar 4, 2025
github-actions bot pushed a commit to carolynzech/rust that referenced this issue Mar 4, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 4, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Mar 6, 2025
github-actions bot pushed a commit to thanhnguyen-aws/verify-rust-std that referenced this issue Mar 6, 2025
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this issue Mar 11, 2025
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. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants