Re: [PATCH v5 6/9] rust: kernel: Add greybus abstractions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: greybus: blanket Send and Sync impls on hd::Device<Ctx>
--

commit d45a0cafbb6b7cfe1ad47516461c24f42e8c8a34
Author: Ayush Singh <ayush@beagleboard.org>

rust: kernel: Add greybus abstractions

This commit adds Rust abstractions for the Greybus core, including host
device registration, driver callbacks, and wire protocol definitions.

> diff --git a/rust/kernel/greybus/hd.rs b/rust/kernel/greybus/hd.rs
> new file mode 100644
> index 0000000000000..9e1a5fc645cd7
> --- /dev/null
> +++ b/rust/kernel/greybus/hd.rs
[ ... ]
> +/// A Greybus host device.
> +///
> +/// # Invariants
> +///
> +/// The wrapped value is a valid `struct gb_host_device` created by `gb_hd_create()`, and every
> +/// [`ARef<Device>`](crate::sync::aref::ARef) owns an increment on its reference count.
> +#[repr(transparent)]
> +pub struct Device<Ctx = device::Normal>(Opaque<bindings::gb_host_device>, PhantomData<Ctx>);
> +
> +// SAFETY: `gb_host_device` is reference counted through its embedded `struct device`, which may be
> +// used from any thread.
> +unsafe impl<Ctx> Send for Device<Ctx> {}
> +
> +// SAFETY: `gb_host_device` has its own internal locking, so it is safe to share references to it
> +// across threads.
> +unsafe impl<Ctx> Sync for Device<Ctx> {}

[Severity: Low]
As reported by Sashiko on v4 [1], in rust/kernel/greybus/hd.rs, Device<Ctx>
provides blanket Send and Sync implementations for all Ctx and omits the
Ctx: device::DeviceContext bound on the struct definition.

Because Device<Ctx> also implements AsRef<device::Device<Ctx>>, marking
Device<device::CoreInternal> as Sync would allow sharing a
&Device<device::CoreInternal> reference across threads and obtaining
concurrent &device::Device<device::CoreInternal> references, even though
device::Device<device::CoreInternal> is intentionally !Sync because
set_drvdata() and drvdata_obtain() mutate dev->driver_data without
synchronization.

Other bus device wrappers in the kernel crate (such as device::Device,
serdev::Device, platform::Device, pci::Device, auxiliary::Device, and
usb::Device) bound Ctx: device::DeviceContext on the struct definition and
only implement Send and Sync for Device (Device<device::Normal>) and Sync
for Device<device::Bound>, keeping Core and CoreInternal contexts !Send and
!Sync.

Should Device<Ctx> in rust/kernel/greybus/hd.rs constrain
Ctx: device::DeviceContext and restrict its Send and Sync implementations to
match the other device wrappers?

[1] https://lore.kernel.org/r/20261021-gb-uart-transport-v4-3-c308ce821d45@beagleboard.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-gb-uart-transport-v5-0-f8e326d6561f@beagleboard.org?part=6