Skip to content

It is unsound to accept arbitrary user-constructed Guards #46

Closed
@jonhoo

Description

@jonhoo

As @Amanieu points out in #44 (comment), the current API which lets users supply their own Guard references to our methods is unsound. Specifically, a user can do the following:

use crossbeam_epoch::*;
let map = HashMap::default();
map.insert(42, String::from("hello"), &epoch::pin());

let evil = crossbeam_epoch::Collector::new();
let evil = evil.register();
let oops = map.get(&42, &evil.guard());

map.remove(&42, &epoch::pin());
// at this point, the default collector is allowed to free `"hello"`
// since no-one has the global epoch pinned as far as it is aware.
// `oops` is tied to the lifetime of a Guard that is not a part of
// the same epoch group, and so can now be dangling.
// but we can still access it!
assert_eq!(oops, "hello");

Here is a sketch of a possible solution, largely inspired by @Amanieu's proposal + #45.

First, we add a Collector field to HashMap, and add a constructor that takes a Collector (this can be no_std). The default() and new() constructors use epoch::default_collector().clone() as their Collector. Then, we add a check to every method that takes a &Guard (which is essentially all of them at the moment) that checks that the given Guard's collector matches self.collector (see SkipList::check_guard). This should take care of the safety issues.

We then add a wrapper type that is tied to a single Guard and exposes all the methods (and some more convenience ones like Index) without a Guard argument. This is #45. It should also have two constructors. One that takes a user-supplied Guard (and checks it), and one that does not (and uses epoch::pin). There should be a method to extract the Guard again from the HashMapRef (since it needs to take ownership of it to also have a variant that uses epoch::pin).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions