Skip to content

Fix Ptr inconsistency in {Rc,Arc} #138303

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

Merged
merged 2 commits into from
Mar 16, 2025
Merged

Fix Ptr inconsistency in {Rc,Arc} #138303

merged 2 commits into from
Mar 16, 2025

Conversation

DiuDiu777
Copy link
Contributor

PR Description

This pr aims to address the problem discussed on zulip.

Problem Clarification

As this post presents, the {Rc, Arc}::{in/de-crement_strong_count_/in} do not limit the layout of the memory that ptr points to, while internally Rc::from_raw_in is called directly.

UB doesn't just appear when the strong count is decremented to zero. Miri also detects the UB of out-of-bounds pointer use when increment strong count is called on a pointer with an incorrect layout(shown as below).

use std::rc::Rc;
#[repr(align(8))]
struct Aligned8(u64);

#[repr(align(16))]
struct Aligned16(u64);

fn main() {
    let rc: Rc<Aligned8> = Rc::new(Aligned8(42));
    let raw_ptr = Rc::into_raw(rc);

    unsafe {
        Rc::increment_strong_count(raw_ptr as *const Aligned16);
    }
}

Miri output:

error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 32 bytes of memory, but got alloc954 which is only 24 bytes from the end of the allocation

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 10, 2025
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 15, 2025

📌 Commit fa183ad has been approved by Mark-Simulacrum

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 Mar 15, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#133055 (Expand `CloneToUninit` documentation.)
 - rust-lang#137147 (Add exclude to config.toml)
 - rust-lang#137864 (Don't drop `Rvalue::WrapUnsafeBinder` during GVN)
 - rust-lang#137890 (doc: clarify that consume can be called after BufReader::peek)
 - rust-lang#137956 (Add RTN support to rustdoc)
 - rust-lang#137968 (Properly escape regexes in Python scripts)
 - rust-lang#138082 (Remove `#[cfg(not(test))]` gates in `core`)
 - rust-lang#138275 (expose `is_s390x_feature_detected!` from `std::arch`)
 - rust-lang#138303 (Fix Ptr inconsistency in {Rc,Arc})
 - rust-lang#138309 (Add missing doc for intrinsic (Fix PR135334))
 - rust-lang#138323 (Expand and organize `offset_of!` documentation.)
 - rust-lang#138329 (debug-assert that the size_hint is well-formed in `collect`)
 - rust-lang#138465 (linkchecker: bump html5ever)
 - rust-lang#138471 (Clean up some tests in tests/ui)
 - rust-lang#138472 (Add codegen test for rust-lang#129795)
 - rust-lang#138484 (Use lit span when suggesting suffix lit cast)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7112951 into rust-lang:master Mar 16, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 16, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2025
Rollup merge of rust-lang#138303 - DiuDiu777:rc-fix, r=Mark-Simulacrum

Fix Ptr inconsistency in {Rc,Arc}

### PR Description
This pr aims to address the problem discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Inconsistency.20in.20.7BRc.2CArc.7D's.20ptr.20requirements/with/504259637).

### Problem Clarification
As this post presents, the `{Rc, Arc}::{in/de-crement_strong_count_/in}` do not limit the layout of the memory that `ptr` points to, while internally `Rc::from_raw_in` is called directly.

UB doesn't just appear when the strong count is decremented to zero. Miri also detects the UB of `out-of-bounds pointer use` when increment strong count is called on a pointer with an incorrect layout(shown as below).

```rust
use std::rc::Rc;
#[repr(align(8))]
struct Aligned8(u64);

#[repr(align(16))]
struct Aligned16(u64);

fn main() {
    let rc: Rc<Aligned8> = Rc::new(Aligned8(42));
    let raw_ptr = Rc::into_raw(rc);

    unsafe {
        Rc::increment_strong_count(raw_ptr as *const Aligned16);
    }
}
```

Miri output:
```
error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 32 bytes of memory, but got alloc954 which is only 24 bytes from the end of the allocation
```
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
Fix Ptr inconsistency in {Rc,Arc}

### PR Description
This pr aims to address the problem discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Inconsistency.20in.20.7BRc.2CArc.7D's.20ptr.20requirements/with/504259637).

### Problem Clarification
As this post presents, the `{Rc, Arc}::{in/de-crement_strong_count_/in}` do not limit the layout of the memory that `ptr` points to, while internally `Rc::from_raw_in` is called directly.

UB doesn't just appear when the strong count is decremented to zero. Miri also detects the UB of `out-of-bounds pointer use` when increment strong count is called on a pointer with an incorrect layout(shown as below).

```rust
use std::rc::Rc;
#[repr(align(8))]
struct Aligned8(u64);

#[repr(align(16))]
struct Aligned16(u64);

fn main() {
    let rc: Rc<Aligned8> = Rc::new(Aligned8(42));
    let raw_ptr = Rc::into_raw(rc);

    unsafe {
        Rc::increment_strong_count(raw_ptr as *const Aligned16);
    }
}
```

Miri output:
```
error: Undefined Behavior: out-of-bounds pointer use: expected a pointer to 32 bytes of memory, but got alloc954 which is only 24 bytes from the end of the allocation
```
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

4 participants