On Tue May 19, 2026 at 11:39 AM BST, Danilo Krummrich wrote: > On Tue May 19, 2026 at 6:52 AM CEST, Eliot Courtney wrote: >> Is this really sound without a covariance proof? For example, with this >> version you could stash a Cell<Option<&'bound pci::Device<Bound>> (even >> with Gary's suggested Core<'_> change) and then observe that reference >> on Drop of Data, which seems unsound to me. > > The Core<'_> change seems unrelated, Core is not Sync, so you can't store it > in > the first place. > > Otherwise, I don't see how one can exploit this. The formal invariance of > Cell<Option<&'bound pci::Device<Bound>> is not practically exploitable because > the framework controls what lifetimes are available and everything reachable > through 'bound outlives the data; something that is shorter lived than 'bound > can't be stored either.
The current code is problematic as the `unbind` callback have `Pin<&'bound Self::Data<'bound>>`. I think we've discussed this last week and the conclusion is that this isn't sound, and the callback should have `Pin<&Self::Data<'bound>>` instead (the outer lifetime should be callback-scoped)`. Did you forget to make the change? > > Bringing back ForLt for this reason seems undesirable, as it has a real design > cost. If we have the `#[variance_check]` macro (like I mentioned in another thread), we could use that to check instead, but it'll still be unsound if lifetime of driver data escapes to devres due to destruction order. Best, Gary
