Re: [PATCH v3 05/27] rust: pci: implement Sync for Device<Bound>

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Implementing `Sync` for `Device<device::Bound>` introduces a soundness hole because it exposes unsynchronized C state mutations via shared references.
--

commit 4843eee61665efed707d8df24917bb38f72f7f5b
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: implement Sync for Device<Bound>

This commit implements Sync for Device<Bound> in addition to Device<Normal>,
allowing drivers to store shared references to bound devices in their
private data while remaining Send.

> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index ffaf05bf9aaaa..c06ac3fb2a64f 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
[ ... ]
>  // SAFETY: `Device` can be shared among threads because all methods of `Device`
>  // (i.e. `Device<Normal>) are thread safe.
>  unsafe impl Sync for Device {}
> +
> +// SAFETY: Same as `Device<Normal>` -- the underlying `struct pci_dev` is the same;
> +// `Bound` is a zero-sized type-state marker that does not affect thread safety.
> +unsafe impl Sync for Device<device::Bound> {}

Does implementing Sync for Device<device::Bound> introduce a data race if
initialization methods are called concurrently?

Device<device::Bound> provides initialization methods like alloc_irq_vectors()
that take a shared reference (&self). If a safe Rust driver spawns multiple
threads and shares this reference, they could concurrently call
alloc_irq_vectors().

This calls the C function pci_alloc_irq_vectors(), which performs lockless
state mutations on the struct pci_dev, such as setting dev->msix_enabled = 1
in msix_capability_init().

Since these lockless mutations are exposed through a shared reference in Rust,
could this lead to state corruption when the device is accessed from multiple
threads at the same time?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517000149.3226762-1-dakr@kernel.org?part=5