[safety] Weaken lifetime in get_xde_state()

[daniel-levin]

The pointer we get back from ddi_get_driver_private assuredly does not have a static lifetime. In practice, this was unlikely to lead to bugs. But, by weakening the promise we make to the compiler, we can prevent unsound optimizations before the compiler makes them, or before we accidentally induce the compiler to make them.

[FelixMcFelix]

I agree with this in principle, but I'm not sure that it helps in the worst case? I.e., any caller can still invoke let state: &'static _ = get_xde_state(); and get a static reference. If the compiler typically picks a narrower 'a when this isn't specified, that might suffice though.

I'm otherwise not sure what the best way is to encode the idea that this pointer's validity does outlive the scope of almost every method within the module. I.e., when the module is not attached I'd expect everything else to be functionally inert.

[daniel-levin]

This is a good point... I think it may be possible to constrain the lifetime to non-static.

[iximeow]

this is effectively the same problem as having a reference to "library-local" data in a Rust-originated .so in userland (which is kind of an Unresolved Problem, lots of proposals for a "dynamic" lifetime etc to describe this, but none of it really has gone anywhere).

strictly speaking, returning &'static XdeState is not itself undefined behavior, but using it while xde_dip is deallocated is. so a more narrow lifetime which still does not exactly track with the initialized-ness of xde_dip could still have the problem. realistically what would be "ideal" is to have some handle to the library provided as an argument to all the xde functions which you could handle.get_xde_state() to get an appropriately-scoped lifetime and know you're not leaking a reference of xde_dip into some external code where it will get used after the module might be unloaded. as long as you never unload xde the 'static is fine, even! (ish. I think we all assume that nothing else calls functions in xde while we're going through xde_attach)

the next best thing would probably be to have an unsafe fn xde_handle() -> XdeHandle that acts as a witness that the library is loaded (where you disclaim that the library is initialized when calling it). then XdeHandle::get_xde_state() would be a more "normal" get_xde_state(&self) -> &XdeState, where the lifetime is tied to the ZST handle thingy so it can't get automagically expanded incorrectly. that ZST should probably not be copy or clone, since you wouldn't want to inadvertently leak those "witnesses" of the module around.

finally you'd want to interlock XdeHandle existing anywhere and xde_detach(). if any XdeHandle would be living on detach that's a bug anyway. that looks like it is an existing bug: if some thread is in xde_ioc_iopte_command, nothing has management_lock, so you could go detach the module, null out xde_dip, and have the system die on the null deref right?

how much of the use of xde_dip is in hot paths, would it be reasonable to have a refcount as another static here, bumped on xde_handle() and decremented on drop? then you could guard detach on that?