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

Box syntax and generator_clone can lead to double free #105084

Closed
cjgillot opened this issue Nov 30, 2022 · 6 comments
Closed

Box syntax and generator_clone can lead to double free #105084

cjgillot opened this issue Nov 30, 2022 · 6 comments
Labels
A-coroutines Area: Coroutines C-bug Category: This is a bug. F-coroutine_clone `#![feature(coroutine_clone)]` F-coroutines `#![feature(coroutines)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cjgillot
Copy link
Contributor

cjgillot commented Nov 30, 2022

I tried this code:

#![feature(generators)]
#![feature(generator_clone)]
#![feature(generator_trait)]
#![feature(box_syntax)]

use std::pin::Pin;
use std::ops::Generator;

fn copy<T: Copy>(x: T) -> T { x }

fn main() {
    let mut g = || {
        // This is desuraged as 4 stages:
        // - allocate a `*mut u8` with `exchange_malloc`;
        // - create a Box that is ignored for trait computations;
        // - compute fields (and yields);
        // - assign to `t`.
        let t = box (5, yield);
        drop(t);
    };
    // Allocate the temporary box.
    Pin::new(&mut g).resume(());
    // The temporary box is in generator locals.
    // As it is not taken into account for trait computation,
    // the generator is `Copy`.
    let mut h = copy(g);
    // We now have 2 boxes with the same backing allocation:
    // one inside `g` and one inside `h`.
    // Proceed and drop `t` in `g`.
    Pin::new(&mut g).resume(());
    // Proceed and drop `t` in `h` -> double free!
    Pin::new(&mut h).resume(());
}

Playground

I expected to see this happen: compilation fails.

Instead, this happened: double free

   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 0.68s
     Running `target/debug/playground`
free(): double free detected in tcache 2
timeout: the monitored command dumped core

Note: if we do not resume the generators after the copy, we get a memory leak.

Meta

rustc version: tested on playground nightly on 2022-11-30.

@cjgillot cjgillot added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. A-coroutines Area: Coroutines F-coroutines `#![feature(coroutines)]` requires-nightly This issue requires a nightly compiler in some way. F-coroutine_clone `#![feature(coroutine_clone)]` labels Nov 30, 2022
@compiler-errors compiler-errors added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 30, 2022
@cjgillot
Copy link
Contributor Author

cjgillot commented Dec 4, 2022

Note: the same bug can be used to manufacture Send/Sync bounds too. Copy was just the easiest to demonstrate.

@calleum
Copy link

calleum commented Feb 2, 2023

@cjgillot is this one closed by #101692?

@cjgillot
Copy link
Contributor Author

cjgillot commented Feb 3, 2023

No. #101692 introduces a new feature, and does not fix the bug the current behaviour.

@jhpratt
Copy link
Member

jhpratt commented Mar 21, 2023

@cjgillot With box syntax removed as of #108471, should this be closed?

@cjgillot
Copy link
Contributor Author

No. Box syntax is just a way to exhibit the issue. The unsoundness persists, but is just a bit harder to trigger.

@jackh726
Copy link
Member

This was fixed in #107421. Whoop @cjgillot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-coroutines Area: Coroutines C-bug Category: This is a bug. F-coroutine_clone `#![feature(coroutine_clone)]` F-coroutines `#![feature(coroutines)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants