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

Coroutines can be assumed to be 'static despite the resume arg not being such. #132104

Closed
danielhenrymantilla opened this issue Oct 24, 2024 · 3 comments · Fixed by #132151
Closed
Assignees
Labels
A-coroutines Area: Coroutines C-bug Category: This is a bug. 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.

Comments

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Oct 24, 2024

I tried this code:

#![forbid(unsafe_code)]
#![feature(coroutine_trait, coroutines)]

use ::core::ops::Coroutine;
use ::std::{thread, time};

fn demo<'not_static>(s: &'not_static str) -> thread::JoinHandle<()> {
    let mut generator = Box::pin({
        #[coroutine]
        move |_ctx| {
            let ctx: &'not_static str = yield;
            yield;
            dbg!(ctx);
        }
    });
    // PROBLEM: `typeof(generator) : 'static`!
    let _assert_usable_for_static: &dyn ::core::any::Any = &generator;

    // exploit:
    generator.as_mut().resume("");
    generator.as_mut().resume(s); // <- generator hoards it as `let ctx`.
    thread::spawn(move || {
        thread::sleep(time::Duration::from_millis(200));
        generator.as_mut().resume(""); // <- resumes from the last `yield`, running `dbg!(ctx)`.
    })
}

fn main() {
    let local = String::from("...");
    let thread = demo(&local);
    drop(local);
    let _unrelated = String::from("UAF");
    thread.join().unwrap();
}

I expected to see this happen:

Compilation error because 'not_static : 'static would be needed for that spawn() (or Any) to pass.

Instead, this happened:

No compilation error, thus unsoundness ensues (coroutine prints UAF in the current playground, thereby proving it is reading unrelated/UAF memory. Miri reports UB).

Meta

  • rustc --version --verbose:

    2024-10-23 4f2f477fded0a47b21ed
    

@rustbot label +I-unsound +A-coroutines +F-coroutines +F-coroutine_trait +requires-nightly

@danielhenrymantilla danielhenrymantilla added the C-bug Category: This is a bug. label Oct 24, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 24, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 24, 2024

Error: Parsing relabel command in comment failed: ...' label add' | error: a label delta at >| ': +I-unsou'...

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@Noratrieb Noratrieb added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness A-coroutines Area: Coroutines F-coroutines `#![feature(coroutines)]` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 24, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 24, 2024
@Noratrieb Noratrieb added requires-nightly This issue requires a nightly compiler in some way. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 24, 2024
@compiler-errors
Copy link
Member

Reduced a bit:

#![forbid(unsafe_code)]
#![feature(coroutine_trait, coroutines)]

use std::ops::Coroutine;
use std::pin::Pin;

fn demo<'not_static>(s: &'not_static str) -> Pin<Box<impl Coroutine<&'not_static str> + 'static>> {
    let mut generator = Box::pin({
        #[coroutine]
        move |ctx: &'not_static str| {
            yield;
            dbg!(ctx);
        }
    });
    generator.as_mut().resume(s);
    generator
}

fn main() {
    let local = String::from("...");
    let mut coro = demo(&local);
    drop(local);
    let _unrelated = String::from("UAF");
    coro.as_mut().resume("");
}

@compiler-errors
Copy link
Member

Given that I fixed #119563, I can also put up a fix for this too. I see where to fix, just need to write up justification for why it's sufficient.

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 28, 2024
…utlives, r=spastorino

Ensure that resume arg outlives region bound for coroutines

When proving that `{Coroutine}: 'region`, we must also prove that the coroutine's resume ty outlives that region as well. See the inline comment.

Fixes rust-lang#132104
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 29, 2024
…utlives, r=spastorino

Ensure that resume arg outlives region bound for coroutines

When proving that `{Coroutine}: 'region`, we must also prove that the coroutine's resume ty outlives that region as well. See the inline comment.

Fixes rust-lang#132104
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 29, 2024
…utlives, r=spastorino

Ensure that resume arg outlives region bound for coroutines

When proving that `{Coroutine}: 'region`, we must also prove that the coroutine's resume ty outlives that region as well. See the inline comment.

Fixes rust-lang#132104
@bors bors closed this as completed in 5dc3639 Oct 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2024
Rollup merge of rust-lang#132151 - compiler-errors:coroutine-resume-outlives, r=spastorino

Ensure that resume arg outlives region bound for coroutines

When proving that `{Coroutine}: 'region`, we must also prove that the coroutine's resume ty outlives that region as well. See the inline comment.

Fixes rust-lang#132104
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-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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants