Require KernelModule to be Sync

[alex]

No description provided.

[geofft]

Seems promising that this requires no other changes.

Can you add a comment defending

unsafe impl<T: SysctlStorage> Sync for Sysctl<T> {}

? I think the short argument is that the only public methods are constructor, destructor, and get() which returns an &T and we know T: Sync. (But this is probably worth noting if we decide to add other methods, so maybe also move it right next to the impl.)

[geofft]

Since it took me a bit to remember why: a KernelModule is basically a global, since it persists across multiple syscall contexts. Therefore, we don't want anything inside a KernelModule to be data-race-unsafe if accessed simultaneously by multiple (kernel) threads: anything that is !Sync (e.g. raw pointers) needs to either be scoped to the lifetime of a syscall and not shared, or needs to be put inside some wrapper structure that handles thread safety and adds Sync.

We aren't strictly depending on this property for correctness (i.e., no APIs exposed in this crate directly make a KernelModule accessible) but it helps us validate that individual kernel modules / APIs exposed in this crate aren't putting things in places where they shouldn't be. (For instance, it forced us to reason out that Sysctl: Sync is sound, despite it containing a raw pointer and therefore not automatically deriving Sync.)

[geofft]

@jason-ni - I see you dropped this in jason-ni/linux-kernel-module-rust@e7e4044, do you remember why you needed to do that? (The build is fine on master now without dropping it but maybe it was for something local you were working on?)