You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 singleGuard 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).
The text was updated successfully, but these errors were encountered:
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:Here is a sketch of a possible solution, largely inspired by @Amanieu's proposal + #45.
First, we add a
Collector
field toHashMap
, and add a constructor that takes aCollector
(this can beno_std
). Thedefault()
andnew()
constructors useepoch::default_collector().clone()
as theirCollector
. 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 givenGuard
's collector matchesself.collector
(seeSkipList::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 likeIndex
) without aGuard
argument. This is #45. It should also have two constructors. One that takes a user-suppliedGuard
(and checks it), and one that does not (and usesepoch::pin
). There should be a method to extract theGuard
again from theHashMapRef
(since it needs to take ownership of it to also have a variant that usesepoch::pin
).The text was updated successfully, but these errors were encountered: