Spinlock is Unsound

[tlambertz]

RustyHermits Spinlock<T> implementation is unsound, since it always implements Sync + Send, no mater what T implements. This behavior was introduced in cf4e955.

Contrary to the commit message, I think this behavior is unsound, since it allows the following to compile:

pub static GLOBAL_WHICH_IS_SEND_AND_SYNC: Option<Spinlock<Rc<u64>>> = None;

This is just an example, but there are ways to construct a non-oneliner of this without the Option<>. Now consider the following:

• Thread A unlocks the Spinlock, calls Rc.clone(), locks Spinlock again
• Thread B does the same
• now both Threads have a reference to 'the same' Rc, which is explicitly neither Send nor Sync (https://doc.rust-lang.org/std/rc/struct.Rc.html#impl-Send), so should not be able to exist across threads. Using it will now lead to race conditions.

Reverting the mentioned commit introduces several issues:

53 | static PCI_DRIVERS: SpinlockIrqSave<Vec<PciDriver>> = SpinlockIrqSave::new(Vec::new());                                                                                                                                       
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `alloc::rc::Rc<core::cell::RefCell<arch::x86_64::kernel::virtio_net::VirtioNetDriver>>` cannot be sent between threads safely         

19 | static PHYSICAL_FREE_LIST: SpinlockIrqSave<FreeList> = SpinlockIrqSave::new(FreeList::new());                                                                                                                                 
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `alloc::rc::Rc<core::cell::RefCell<collections::doublylinkedlist::Node<mm::freelist::FreeListEntry>>>` cannot be sent between th

14 | static KERNEL_FREE_LIST: SpinlockIrqSave<FreeList> = SpinlockIrqSave::new(FreeList::new());
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `alloc::rc::Rc<core::cell::RefCell<collections::doublylinkedlist::Node<mm::freelist::FreeListEntry>>>` cannot be sent between thre

I am currently not sure how to best resolve this issue. Maybe tag the affected structs with unsafe impl Send for FreeList {}. Though that might be dangerous aswell, since it essential is just a DoublyLinkedList which can generate Rc clones. As far as I can see, there are no races currently possible. But that is only because the methods that could race are not used 'by accident', and not because the compiler prohibits this.
KERNEL_FREE_LIST.lock().list.push(entry); is used, so why not KERNEL_FREE_LIST.lock().list.head();, which returns Option<Rc<RefCell<Node<T>>>>

My current workaround is manually marking the structs I protect with a spinlock as Send, so I get the appropriate compiler guarantees without having to fix this issue. I initially constructed something similar to the example above and was wondering why it compiled.

[stlankes]

You are right cf4e955 is wrong. Let me look in the implementation...

[stlankes]

Issue is solved with PR #97