Data race allowed on K of ParallelEventEmitter<K>

[JOE1994]

Hello,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

parallel-event-emitter/src/lib.rs

Line 237 in e9e2a75

unsafe impl<K: EventKey> Sync for Inner<K> {}

Inner<K> implements Sync for all K: EventKey, even when K: !Sync.
It is possible to create a data race to K: !Sync using ParallelEventEmitter::event_names_visitor(), which may lead to undefined behavior.

Reproduction

Below is an example program that exhibits undefined behavior using safe APIs of parallel-event-emitter.

Show Detail

#![forbid(unsafe_code)]
use parallel_event_emitter::ParallelEventEmitter;

use std::cell::Cell;
use std::hash::{Hash, Hasher};
use std::sync::Arc;

// A simple tagged union used to demonstrate problems with data races in Cell.
#[derive(Debug, Hash, PartialEq, Eq, Clone, Copy)]
enum RefOrInt {
    Ref(&'static u64),
    Int(u64),
}
static SOME_INT: u64 = 123;

#[derive(PartialEq, Eq, Clone)]
struct Foo(Cell<RefOrInt>);

impl Hash for Foo {
    fn hash<H: Hasher>(&self, state: &mut H) {
        self.0.get().hash(state);
    }
}

fn main() {
    let non_sync_key = Foo(Cell::new(RefOrInt::Ref(&SOME_INT)));
    let mut emit0 = ParallelEventEmitter::new();
    emit0.add_listener(
        non_sync_key,
        || Ok(()) // dummy listener
    );
    let emit0 = Arc::new(emit0);

    let emit1 = emit0.clone();
    std::thread::spawn(move || {
        let emit1 = emit1;

        emit1.event_names_visitor(|key: &Foo| {
            loop {
                // Repeatedly write Ref(&addr) and Int(0xdeadbeef) into the cell.
                key.0.set(RefOrInt::Ref(&SOME_INT));
                key.0.set(RefOrInt::Int(0xdeadbeef));
            }
        });
    });

    emit0.event_names_visitor(|key: &Foo| {
        loop {
            if let RefOrInt::Ref(addr) = key.0.get() {
                // Hope that between the time we pattern match the object as a
                // `Ref`, it gets written to by the other thread.
                if addr as *const u64 == &SOME_INT as *const u64 {
                    continue;
                }

                println!("Pointer is now: {:p}", addr);
                println!("Dereferencing addr will now segfault: {}", *addr);
            }
        }
    });
}

Output:

Pointer is now: 0xdeadbeef

Terminated with signal 11 (SIGSEGV)

Tested Environment

• Crate: parallel-event-emitter
• Version: 0.2.4
• OS: Ubuntu 18.04.5 LTS
• Rustc version: rustc 1.50.0 (cb75ad5db 2021-02-10)