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

Evade borrowck by wrapping your lifetime crimes in a simple closure #112056

Closed
cormac-ainc opened this issue May 29, 2023 · 8 comments · Fixed by #112070
Closed

Evade borrowck by wrapping your lifetime crimes in a simple closure #112056

cormac-ainc opened this issue May 29, 2023 · 8 comments · Fixed by #112070
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@cormac-ainc
Copy link

The following code compiles, since February nightlies and on stable version 1.69 (no older stable versions are affected). (playground):

struct Spooky<'b> {
    owned: String,
    borrowed: &'b str,
}

impl<'b> Spooky<'b> {
    fn create_self_reference<'a>(&'a mut self) {
        let mut closure = || {
            let borrow: &'a String = &self.owned;
            self.borrowed = borrow;
        };
        closure();
    }
}

fn main() {
    let mut spooky: Spooky<'static> = Spooky {
        owned: "Hello".to_string(),
        borrowed: "static string, initially",
    };
    spooky.create_self_reference();
    std::mem::take(&mut spooky.owned);
    for _ in 0..1000 {
        let _ = format!("askld");
    }
    println!("{}", spooky.borrowed);
}

This is clearly a use-after-free. On my Linux machine it prints a garbage string. It should not compile at all, the error should be basically the same as if you didn't wrap the offending lines in a closure. The issue still occurs with self.borrowed = &self.owned, the annotations are just for clarity.

The problem was introduced in #107969.

Hat tip to @ladecaz for writing the cursed code that got us here.

cargo-bisect-rustc results

searched nightlies: from nightly-2021-04-25 to nightly-2023-05-29
regressed nightly: nightly-2023-02-21
searched commit range: 7aa413d...5243ea5
regressed commit: e7eaed2

bisected with cargo-bisect-rustc v0.6.6

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --start=2021-04-25 --regress success
@cormac-ainc cormac-ainc added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels May 29, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 29, 2023
@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented May 29, 2023

@rustbot label I-unsound regression-from-stable-to-stable -regression-untriaged

@rustbot rustbot added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-untriaged Untriaged performance or correctness regression. labels May 29, 2023
@compiler-errors compiler-errors added the T-types Relevant to the types team, which will review and decide on the PR/issue. label May 29, 2023
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 29, 2023
@Noratrieb
Copy link
Member

cc @b-naber @lcnr

@lcnr
Copy link
Contributor

lcnr commented May 29, 2023

uh, that's not good, looking into it

@lcnr
Copy link
Contributor

lcnr commented May 29, 2023

a smaller guaranteed segfault, needs --edition=2021

struct Spooky<'b> {
    owned: Option<&'static u32>,
    borrowed: &'b &'static u32,
}

impl<'b> Spooky<'b> {
    fn create_self_reference<'a>(&'a mut self) {
        let mut closure = || {
            if let Some(owned) = &self.owned {
                let borrow: &'a &'static u32 = owned;
                self.borrowed = borrow;
            }
        };
        closure();
    }
}

fn main() {
    let mut spooky: Spooky<'static> = Spooky {
        owned: Some(&1),
        borrowed: &&1,
    };
    spooky.create_self_reference();
    spooky.owned = None;
    println!("{}", **spooky.borrowed);
}

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 30, 2023
…r=oli-obk

change `BorrowKind::Unique` to be a mutating `PlaceContext`

fixes rust-lang#112056

I believe that `BorrowKind::Unique` is a footgun in general, so I added a FIXME and opened rust-lang#112072. This is a bit too involved for this PR though.
@bors bors closed this as completed in e6e4f7e May 31, 2023
@lcnr
Copy link
Contributor

lcnr commented May 31, 2023

keeping this open until beta (and maybe stable) backport

@lcnr lcnr reopened this May 31, 2023
@riking
Copy link

riking commented Jun 2, 2023

Stable backport complete, beta backport pending.

@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2023

fixed everywhere now

@lcnr lcnr closed this as completed Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-critical Critical priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
8 participants