Dereferencing raw pointer causes use-after-free

Why does using the pointer from AtomicPtr cause a use-after-free? Is the pointer created in new() freed at the end of new()?

use std::sync::atomic::{AtomicPtr, Ordering};
fn main() {
    let x = new();
    let data = unsafe {
        *x.load(Ordering::Acquire)
    };
    println!("{data}")
}

fn new() -> AtomicPtr<usize> {
    let mut data = 101usize;
    AtomicPtr::new(&mut data)
}

Miri gives the following output:

~/C/r/rptr (main) > cargo miri run                                                                                                                                                                                                                            (base) 
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/bin/cargo-miri target/miri/x86_64-apple-darwin/debug/rptr`
error: Undefined Behavior: pointer to alloc1068 was dereferenced after this allocation got freed
 --> src/main.rs:5:9
  |
5 |         *x.load(Ordering::Acquire)
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ pointer to alloc1068 was dereferenced after this allocation got freed
  |
  = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
          
  = note: inside `main` at src/main.rs:5:9
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:122:18
  = note: inside closure at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
  = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
  = note: inside closure at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
  = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
  = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
  = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
  = note: inside `std::rt::lang_start_internal` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
  = note: inside `std::rt::lang_start::<()>` at /Users/littlegoombas/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:144:17

error: aborting due to previous error

Thank you, I'm just getting started with unsafe code.

It's nothing to do with AtomicPtr per se. You created a pointer to something on the stack in new, and that pointer became dangling once you returned from new. Then you dereferenced the dangling pointer.

(It was allocated on the stack using Miri's terminology, I guess.)

Here's the same thing without AtomicPtr. (You can run Miri in the Playground under Tools.)

3 Likes

Ahh ok, thank you!

Is the reason this would work with a reference because the borrow-checker verifies everything is ok, but it can't do that with raw pointers?

Here's a version with references: Rust Playground

Also, does this mean everything on a function's stack frame is destroyed once the function returns, except for if something needs to live longer?

In any case, I'm writing a data structure where I need to compare-and-swap pointers, so would the only way to do this be by using Box::into_raw() and Box::from_raw()?

Thank you so much! And feel free to correct any misconceptions you see :slight_smile:

Well, there's a few things going on here. Let's cover why the playground example works first. It's not because of a distinction between references and raw pointers, it's because of a distinction between static values and values on the stack.

Rust has a feature called "promotion" that allows literals in functions (among other things) to automatically be stored into a static location, to which you can create &'static references. That's what's happening in your playground -- you're taking a &'static inside the function body, and that can coerce to any other lifetime as the return value.

If you make the value get stored in a local variable explicitly, it no longer compiles.

Everything on the function's stack frame is destroyed once the function returns. There's no exception for "things that need to live longer" per se, but some things might be able to be promoted to static storage so you can return a reference to them. If you need something produced in the function to live longer, you need to

  • Have it be statically stored, or
  • Return it, or
  • Put it on the heap (e.g. in a Box) and return the container (e.g. the Box), or
  • Put it on the heap, leak the memory, and return a pointer to the leaked memory, or
  • Put it in a container or location the caller provides (e.g. &mut Vec), or
  • Some other niches like thread local storage, etc

But you can't leave it on the stack and have it persist when you return.


So what about raw pointers? If you return a raw pointer to the promotable value as before, there's no problem (run it through Miri to see). But if you use the local variable, it compiles even though it has undefined behavior (as Miri detects). The reason that it was allowed to compile is two-fold:

  1. Raw pointers don't have the same validity constraints as references. They can point to deallocated memory, memory that doesn't contain a valid value (like a bool containing 42), they can be unaligned, whatever. None of this is considered unsafe if all you do is carry the pointer around, so it was fine to return a *const to a stack location immediately invalidated. However, you can't actually read the value behind the pointer in safe code, leading into...

  2. You used unsafe to dereference the returned pointer. In safe code, Rust upholds various guarantees, such as the validity of references; these guarantees provide memory safety, data race freedom, and so on. That's why the example returning a local reference failed. But when you use unsafe, you're taking on the responsibility of upholding Rust's memory safety guarantees. You're telling the compiler "don't worry, I've got this!" If you make a mistake, the result can be undefined behavior and memory unsafety, as happened here. In a sense, you asserted that the *const pointed to a valid value, and the compiler took your word for it, but it did not.

There's not enough information for me to give very good advice here. You should probably make sure you have handle on the concept of ownership in Rust. Your data is going to have to need to have an owner somewhere; maybe in a Box, or in memory leaked from a Box, behind some sort of reference counting, in an arena... there are many possibilities depending on the use case. But there's no built in garbage collection.

Your use case may or may not be possible using safe Rust. Especially when starting out, I encourage you to look for a way to do it in safe Rust first, and if it's not possible to look into well tested crates that enable you to do it through a safe API.

5 Likes

Thank you for such a detailed explanation; I understand what is happening with the promotion now.

The data structure I'm writing is the lock-free vector from this paper. Although I could probably produce a better and safer implementation with external crates, I want to implement the vector all the way down, so I really understand what goes into it.

The paper doesn't specify a memory reclamation scheme, so I'm going to take some liberties there. I don't want to do reference counting because I'm already familiar with it, and I'm leaning towards using hazard pointers because it seems like a cool technique and Jon Gjenset has series of streams about them.

If you have any advice, I'd appreciate it, but I also understand that would beyond the scope of this thread. Where's crossbeam when you need it! :slight_smile:

If you are writing a sensible, well-behaved data structure, then with high probability, you don't need (and shouldn't use) any unsafe for implementing it. For storing individually heap-allocated elements, just use the saafe API of Box without raw pointers. Have a look at the safe functions in std::mem, eg. swap, replace, and take.

There is no need to perform complicated trickery – it basically never turns out well unless you are already an expert in the language. The whole point of Rust's ownership-and-borrowing model is that it fits real-life use cases very well, and you can do 99% of what's considered correct and memory-safe without ever touching a raw pointer or managing memory manually. When in doubt, try to do it in safe code, instead of assuming that you need unsafe.

4 Likes

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.