Skip to content

Pin::new_unchecked's stated safety condition is not quite stringent enough to ensure soundness #147794

Description

@ais523

Location (URL)

https://doc.rust-lang.org/std/pin/struct.Pin.html#method.new_unchecked

Summary

The following code has undefined behaviour, but (with reasonable interpretations) obeys all the stated safety conditions on its one unsafe call, to Pin::new_unchecked():

use core::cell::Cell;
use core::future::poll_fn;
use core::ops::{Deref, DerefMut};
use core::pin::Pin;
use core::task::{Poll, Context, Waker};

struct Malicious<'a, T> {
    fixed: Box<T>, 
    movable: Box<T>,
    comm: &'a Cell<Box<T>>,
    which: &'a Cell<bool>,
}

impl<'a, T> Deref for Malicious<'a, T> {
    type Target = T;
    fn deref(&self) -> &T {
        if self.which.get() {
            &*self.movable
        } else {
            &*self.fixed
        }
    }
}
impl<'a, T> DerefMut for Malicious<'a, T> {
    fn deref_mut(&mut self) -> &mut T {
        if self.which.get() {
            Cell::from_mut(&mut self.movable).swap(self.comm);
            &mut *self.movable
        } else {
            &mut *self.fixed
        }
    }
}

async fn future_maker(ready: &Cell<Poll<()>>) {
    let s = String::from("Hello, world!");
    let sref = &s;
    let pf = poll_fn(move |_| ready.get());
    pf.await;
    println!("{}", sref);
}

fn main() {
    let ready = Cell::new(Poll::Pending);
    let comm = Cell::new(Box::new(future_maker(&ready)));
    let which = Cell::new(false);
    let m = Malicious {
        fixed: Box::new(future_maker(&ready)),
        movable: Box::new(future_maker(&ready)),
        comm: &comm,
        which: &which
    };

    // Safety: m is a reference to *m.fixed, and *m.fixed is not
    // moved out of until it is dropped; m's Drop, Deref,
    // DerefMut methods do not move out of the reference target
    let mut pin = unsafe { Pin::new_unchecked(m) };

    // Pin::new_unchecked does not require that the reference
    // always points to the same place; this can be exploited
    // to change the reference to point to *m.movable, then
    // move out of *m.movable, causing a use-after-free
    let mut noop_context = Context::from_waker(Waker::noop());
    which.set(true);
    // as_mut() derefs to what was, before this command, comm
    assert_eq!(pin.as_mut().poll(&mut noop_context), Poll::Pending);
    // move the box that was dereffed back into comm
    pin.as_mut();
    // move the future into a different box
    let mut old_box = comm.replace(Box::new(future_maker(&ready)));
    let future = std::mem::replace(&mut *old_box, future_maker(&ready));
    let new_box = Box::new(future);
    comm.set(new_box);
    // poll the future from its new location (UB because we are
    // polling a moved future)
    ready.set(Poll::Ready(()));
    assert_eq!(pin.as_mut().poll(&mut noop_context), Poll::Ready(()));
}

The documentation for Pin::new_unchecked() should be updated to disallow this case. The problem is that the stated safety conditions on Pin::new_unchecked() are not quite stringent enough – they require that "that the data Ptr points to is pinned", but the condition that is actually required is "all data that Ptr ever points to, including in the future, is pinned"; the documentation doesn't seem to have taken into account the possibility of references that can change what they point to over time. The exploit above works by pointing the reference m.fixed (which is correctly pinned) at the time of the Pin::new_unchecked() call, then changing the reference to point to m.movable (which is not correctly pinned) before polling the future.

The sentence that needs changing is probably

Moreover, by calling this method you promise that the reference Ptr dereferences to will not be moved out of again; in particular, it must not be possible to obtain a &mut Ptr::Target and then move out of that reference (using, for example mem::swap).

The "to obtain a &mut Ptr::Target" part of this sentence is unclear in the case of this sort of changeable-target reference – it should be updated to cover all references that the given pointer ever dereferences to, whereas the current wording refers to either the reference that Ptr currently dereferences to (not restrictive enough to ensure soundness) or all references ever created of type &mut Ptr::Target (which is impossible to comply with and would make the method useless).

This bug report was inspired by #147777, which prompted me to check the Pin documentation to ensure that all the specified safety requirements were sufficient.

Meta

Tested on the Rust playground, 1.92.0-nightly (2025-10-15 57ef8d642d21965304bd).

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsA-pinArea: PinT-libsRelevant to the library team, which will review and decide on the PR/issue.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions