linux-kernel - Re: [PATCH] Move pwm registration into pwm::Chip::new

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: <8711d40496e0e12e3efcd7fd9e11bdea6de68c6d.camel@posteo.de>
Date: Fri, 28 Nov 2025 12:25:04 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Drew Fustini <fustini@...nel.org>, Guo Ren <guoren@...nel.org>, Fu Wei	
 <wefu@...hat.com>, Uwe Kleine-König
 <ukleinek@...nel.org>,  Michal Wilczynski <m.wilczynski@...sung.com>,
 Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
 Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron	 <bjorn3_gh@...tonmail.com>, Benno
 Lossin <lossin@...nel.org>, Andreas Hindborg	 <a.hindborg@...nel.org>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich	 <dakr@...nel.org>,
 linux-riscv@...ts.infradead.org, linux-pwm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] Move pwm registration into pwm::Chip::new

On Fri, 2025-11-28 at 09:28 +0000, Alice Ryhl wrote:
> On Thu, Nov 27, 2025 at 05:15:06PM +0000, Markus Probst wrote:
> > The `pwm::Registration::register` function provides no guarantee that the
> > function isn't called twice with the same pwm chip, which is considered
> > unsafe.
> > 
> > Add the code responsible for the registration into `pwm::Chip::new`. The
> > registration will happen before the driver gets access to the refcounted
> > pwm chip and can therefore guarantee that the registration isn't called
> > twice on the same pwm chip.
> > 
> > Signed-off-by: Markus Probst <markus.probst@...teo.de>
> > ---
> > This patch provides the additional guarantee that the pwm chip doesn't
> > get registered twice.
> > 
> > The following changes were made:
> > - change the visibility of `pwm::Registration` to private
> > - remove the `pwm::Registration::register` function
> > - add code for registering the pwm chip in `pwm::Chip::new`
> > - add Send + Sync bounds to `PwmOps`
> > 
> > Note that I wasn't able to test this patch, due to the lack of hardware.
> 
> Overall looks reasonable, but I have one question:
> 
> > @@ -654,50 +668,23 @@ unsafe fn dec_ref(obj: NonNull<Chip<T>>) {
> >  // structure's state is managed and synchronized by the kernel's device model
> >  // and PWM core locking mechanisms. Therefore, it is safe to move the `Chip`
> >  // wrapper (and the pointer it contains) across threads.
> > -unsafe impl<T: PwmOps + Send> Send for Chip<T> {}
> > +unsafe impl<T: PwmOps> Send for Chip<T> {}
> >  
> >  // SAFETY: It is safe for multiple threads to have shared access (`&Chip`) because
> >  // the `Chip` data is immutable from the Rust side without holding the appropriate
> >  // kernel locks, which the C core is responsible for. Any interior mutability is
> >  // handled and synchronized by the C kernel code.
> > -unsafe impl<T: PwmOps + Sync> Sync for Chip<T> {}
> > +unsafe impl<T: PwmOps> Sync for Chip<T> {}
> 
> Why was this changed?

Registration::register required PwmOps to be Send + Sync.
Prior to this change, Chip::new didn't require it for PwmOps. Meaning
it was possible to allocate a new Chip with a PwmOps that is not Send +
Sync. As there was no use for it and it isn't possible anymore to
allocate a new Chip without registering it, I added Send + Sync as
trait dependency (see 1. hunk of rust/kernel/pwm.rs in the patch).

Because PwmOps now implied Send + Sync, it wasn't necessary anymore to
have the additional bounds there.

Thanks
- Markus Probst

> 
> Alice

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)