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

ReservedSpace is unsound due to not being bound to its origin transaction. #258

Closed
nolanderc opened this issue May 1, 2024 · 0 comments · Fixed by #259
Closed

ReservedSpace is unsound due to not being bound to its origin transaction. #258

nolanderc opened this issue May 1, 2024 · 0 comments · Fixed by #259
Milestone

Comments

@nolanderc
Copy link
Contributor

ReservedSpace is currently defined without lifetimes:

pub struct ReservedSpace {
size: usize,
start_ptr: *mut u8,
written: usize,
}

However, the callback functions in which it is used expose it as FnOnce(&mut ReservedSpace). This means it is possible to write something like the following to modify the reserved space in safe Rust:

let env_foo = heed::Env::open(...)?;
txn_foo = env_foo.write_txn()?;
let db_foo = env1.create_database(&mut txn_foo, None);  

let env_bar = heed::Env::open(...)?;
txn_bar = env_bar.write_txn()?;
let db_bar = env1.create_database(&mut txn_bar, None);  


// reserve an outer space
db_foo.put_reserved(&mut txn_foo, key_foo, 3, |space_foo| {

    // fill the outer space
    space_bar.write_all(b"baz");

    // reserve an inner space
    db_bar.put_reserved(&mut txn_bar, key_bar, 6, |space_bar| {

        // swap them:
        std::mem::swap(space_foo, space_bar);

        // `space_foo` now points into `db_bar`
        // `space_bar` now points into `db_foo`, which is fully written

    })?; // returns Ok(())

    // `space_foo` still points into `db_bar`.

    db_bar.delete(&mut txn_bar, key_bar)?;

    // `space_foo` points to deleted value.

    space_foo.write_all(b"foobar")?; // UNDEFINED BEHAVIOUR!

})?;

This could be fixed by attaching a lifetime to ReservedSpace<'a> which is only valid for the duration of the closure.

nolanderc added a commit to nolanderc/heed that referenced this issue May 1, 2024
nolanderc added a commit to nolanderc/heed that referenced this issue May 1, 2024
@Kerollmops Kerollmops added this to the v0.20.1 milestone May 1, 2024
Kerollmops added a commit that referenced this issue May 1, 2024
add lifetime to `ReservedSpace` (closes #258)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants