linux-kernel - Re: [PATCH 1/2] rust: pci: impl Send + Sync for pci::Device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9rLs9jY1h7IdImw@cassiopeiae>
Date: Wed, 19 Mar 2025 14:50:43 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: bhelgaas@...gle.com, gregkh@...uxfoundation.org, rafael@...nel.org,
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	a.hindborg@...nel.org, tmgross@...ch.edu, linux-pci@...r.kernel.org,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] rust: pci: impl Send + Sync for pci::Device

On Wed, Mar 19, 2025 at 02:30:31PM +0100, Danilo Krummrich wrote:
> On Wed, Mar 19, 2025 at 01:16:52PM +0000, Alice Ryhl wrote:
> > On Wed, Mar 19, 2025 at 01:47:01PM +0100, Danilo Krummrich wrote:
> > > On Wed, Mar 19, 2025 at 12:05:13PM +0000, Alice Ryhl wrote:
> > > > On Tue, Mar 18, 2025 at 10:29:21PM +0100, Danilo Krummrich wrote:
> > > > > Commit 7b948a2af6b5 ("rust: pci: fix unrestricted &mut pci::Device")
> > > > > changed the definition of pci::Device and discarded the implicitly
> > > > > derived Send and Sync traits.
> > > > > 
> > > > > This isn't required by upstream code yet, and hence did not cause any
> > > > > issues. However, it is relied on by upcoming drivers, hence add it back
> > > > > in.
> > > > > 
> > > > > Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> > > > 
> > > > I have a question related to this ... does the Driver trait need to
> > > > require T: Send?
> > > 
> > > The driver trait does not have a generic, it doesn't need one. But I think I
> > > still get what you're asking.
> 
> Turns out I did not. :)
> 
> > Right I mean, should it be:
> > 
> > trait Driver: Send + Sync {
> >     ...
> > }
> 
> Yes, you're absolutely right with this, thanks for pointing this out.

Just to clarify, the reason we need Sync is that we want to be able to access
the driver's private data from an IRQ handler. Otherwise, we can only ever
safely access the driver's private data from bus callbacks, which should be
synchronized by the device' mutex.

> 
> > 
> > > The driver trait never owns a shared reference of the device, it only ever gives
> > > out a reference that the driver core guarantees to be valid.
> > > 
> > > > The change itself LGTM, so:
> > > > 
> > > > Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> > > > 
> > > > Alice